Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Failure] tests/python/topi/python/test_topi_broadcast.py::test_shift under LLVM12 #7539

Closed
mesauser opened this issue Feb 26, 2021 · 5 comments

Comments

@mesauser
Copy link
Contributor

Test failed on latest llvm 12.0

FAILED tests/python/topi/python/test_topi_broadcast.py::test_shift - AssertionError:
E AssertionError:
E Not equal to tolerance rtol=0.0001, atol=0.0001
E
E Mismatched elements: 4 / 4 (100%)
E Max absolute difference: 88
E Max relative difference: inf
E x: array([[[-128, -88],
E [ 32, 8]]], dtype=int8)
E y: array([[[0, 0],
E [0, 0]]], dtype=int8)
python/tvm/testing.py:82: AssertionError

There is a test failed on llvm12.0, but success on llvm-8.

I saw the llvm version in Jenkins test system is still 8.0 while the most recent release is 12.0.
It will not be noticed by developer if the test is still base on older llvm version.
Is there a plan to migrate the test base on latest llvm or multi-versions?

@tqchen
Copy link
Member

tqchen commented Feb 26, 2021

Thanks @mesauser for reporting. We are testing a spectrum of LLVM in the CI. Right now the latest llvm being tested was llvm-10. #7541 moves that to llvm 11. Given LLVM 12 get released recently, we should work to resolve this problem and upgrade the CI.

DO you might to look into it ?

@tqchen tqchen changed the title [Failure] failure tests/python/topi/python/test_topi_broadcast.py::test_shift [Failure] tests/python/topi/python/test_topi_broadcast.py::test_shift under LLVM12 Feb 26, 2021
@mesauser
Copy link
Contributor Author

mesauser commented Mar 1, 2021

The method to legalize/ lower vector shift (like: v8i16 = sra t15, t14 ) changed in llvm X86 backend change (version>10)
It seems not implemented well in this special case for type i8/i16 yet.

Dumped examples for shift right with v8i16 and v4i32 type (llvm12):

Legalizing vector op: t20: v4i32 = sra t19, t16
Trying custom legalization
Vector-legalized selection DAG: %bb.0 'broadcast_binary_right_shift_compute_:entry'
SelectionDAG has 36 nodes:
t0: ch = EntryToken
t2: i64,ch = CopyFromReg t0, Register:i64 %2
t14: i64,ch = load<(load 8 from %ir.3, align 128)> t0, t2, undef:i64
t7: i64,ch = CopyFromReg t0, Register:i64 %1
t17: i64,ch = load<(load 8 from %ir.5, align 128)> t0, t7, undef:i64
t18: v2i64 = scalar_to_vector t17
t19: v4i32 = bitcast t18
t15: v2i64 = scalar_to_vector t14
t28: v8i16 = bitcast t15
t30: v8i16 = vector_shuffle<4,5,6,7,u,u,u,u> t28, undef:v8i16
t12: ch = TokenFactor t14:1, t17:1
t31: v8i16 = vector_shuffle<0,1,1,1,u,u,u,u> t28, undef:v8i16
t35: v4i32 = bitcast t31
t36: v4i32 = X86ISD::VSRA t19, t35
t32: v8i16 = vector_shuffle<2,3,3,3,u,u,u,u> t28, undef:v8i16
t37: v4i32 = bitcast t32
t38: v4i32 = X86ISD::VSRA t19, t37
t43: v4i32 = vector_shuffle<0,u,u,5> t36, t38
t33: v8i16 = vector_shuffle<0,1,1,1,u,u,u,u> t30, undef:v8i16
t39: v4i32 = bitcast t33
t40: v4i32 = X86ISD::VSRA t19, t39
t34: v8i16 = vector_shuffle<2,3,3,3,u,u,u,u> t30, undef:v8i16
t41: v4i32 = bitcast t34
t42: v4i32 = X86ISD::VSRA t19, t41
t44: v4i32 = vector_shuffle<2,u,u,7> t40, t42
t45: v4i32 = vector_shuffle<0,3,4,7> t43, t44
t23: v2i64 = bitcast t45
t25: i64 = extract_vector_elt t23, Constant:i64<0>
t11: i64,ch = CopyFromReg t0, Register:i64 %0
t26: ch = store<(store 8 into %ir.8, align 128)> t12, t25, t11, undef:i64

Legalizing vector op: t16: v8i16 = sra t15, t14
Vector-legalized selection DAG: %bb.0 'broadcast_binary_right_shift_compute_:entry'
SelectionDAG has 51 nodes:
t0: ch = EntryToken
t2: i64,ch = CopyFromReg t0, Register:i64 %2
t14: v8i16,ch = load<(load 16 from %ir.3, align 128, !tbaa !121)> t0, t2, undef:i64
t7: i64,ch = CopyFromReg t0, Register:i64 %1
t15: v8i16,ch = load<(load 16 from %ir.5, align 128, !tbaa !134)> t0, t7, undef:i64
t24: v8i16 = X86ISD::VSHLI t14, TargetConstant:i8<12>
t28: v8i16 = X86ISD::VSRAI t24, TargetConstant:i8<15>
t30: v8i16 = add t24, t24
t33: v8i16 = X86ISD::VSRAI t30, TargetConstant:i8<15>
t35: v8i16 = add t30, t30
t38: v8i16 = X86ISD::VSRAI t35, TargetConstant:i8<15>
t40: v8i16 = add t35, t35
t43: v8i16 = X86ISD::VSRAI t40, TargetConstant:i8<15>
t46: v8i16 = BUILD_VECTOR Constant:i16<-1>, Constant:i16<-1>, Constant:i16<-1>, Constant:i16<-1>, Constant:i16<-1>, Constant:i16<-1>, Constant:i16<-1>, Constant:i16<-1>
t26: v8i16 = X86ISD::VSRAI t15, TargetConstant:i8<8>
t48: v8i16 = and t26, t28
t47: v8i16 = xor t28, t46
t49: v8i16 = and t15, t47
t50: v8i16 = or t48, t49
t32: v8i16 = X86ISD::VSRAI t50, TargetConstant:i8<4>
t52: v8i16 = and t32, t33
t51: v8i16 = xor t33, t46
t53: v8i16 = and t50, t51
t54: v8i16 = or t52, t53
t37: v8i16 = X86ISD::VSRAI t54, TargetConstant:i8<2>
t56: v8i16 = and t37, t38
t55: v8i16 = xor t38, t46
t57: v8i16 = and t54, t55
t58: v8i16 = or t56, t57
t12: ch = TokenFactor t14:1, t15:1
t42: v8i16 = X86ISD::VSRAI t58, TargetConstant:i8<1>
t60: v8i16 = and t42, t43
t59: v8i16 = xor t43, t46
t61: v8i16 = and t58, t59
t62: v8i16 = or t60, t61
t17: v4i32 = bitcast t62
t19: i32 = extract_vector_elt t17, Constant:i64<0>
t11: i64,ch = CopyFromReg t0, Register:i64 %0
t20: ch = store<(store 4 into %ir.8, align 128, !tbaa !147)> t12, t19, t11, undef:i64

@tqchen
Copy link
Member

tqchen commented Mar 3, 2021

I see, Thank you @mesauser for looking into it, seems the solution should be disable the i16 and i8 shift test for now? A PR is more than welcomed

@mesauser
Copy link
Contributor Author

mesauser commented Mar 5, 2021

#7597

tqchen pushed a commit that referenced this issue Mar 5, 2021
#7539

Co-authored-by: guoweijun <guoweijun@baidu.com>
@tqchen tqchen closed this as completed Mar 5, 2021
@tqchen
Copy link
Member

tqchen commented Mar 5, 2021

Thank you @mesauser !

trevor-m pushed a commit to trevor-m/tvm that referenced this issue May 6, 2021
apache#7539

Co-authored-by: guoweijun <guoweijun@baidu.com>
trevor-m pushed a commit to neo-ai/tvm that referenced this issue May 11, 2021
apache#7539

Co-authored-by: guoweijun <guoweijun@baidu.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants