-
Notifications
You must be signed in to change notification settings - Fork 11.9k
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
Miscompilation after InstCombinePass
#74890
Comments
I reduced this to two related miscompilation issues, but happening in different parts in define <4 x i16> @src(ptr align 8 %_0) {
start:
%_5.sroa.5.32.copyload = load <3 x i16>, ptr %_0, align 8
%_5.sroa.5.32.vecblend = shufflevector <3 x i16> <i16 0, i16 undef, i16 undef>, <3 x i16> %_5.sroa.5.32.copyload, <4 x i32> <i32 0, i32 1, i32 3, i32 5>
ret <4 x i16> %_5.sroa.5.32.vecblend
} Alive2: https://alive2.llvm.org/ce/z/3AMDi4 2: define <8 x i16> @src(ptr align 8 %_0, ptr align 8 %_1) unnamed_addr #0 {
start:
%_5.sroa.5.32.copyload = load <8 x i16>, ptr %_1, align 8
%_5.sroa.5.32.vecblend = shufflevector <8 x i16> %_5.sroa.5.32.copyload, <8 x i16> <i16 poison, i16 undef, i16 undef, i16 undef, i16 poison, i16 poison, i16 poison, i16 poison>, <8 x i32> <i32 poison, i32 9, i32 10, i32 11, i32 4, i32 5, i32 6, i32 7>
ret <8 x i16> %_5.sroa.5.32.vecblend
} Alive2: https://alive2.llvm.org/ce/z/Ha-Sga There seems to be no out-of-bound access in both shuffle vector constant, I don't think we are free to miscompile here (which I think it would be the case if we were to access it out-of-bound), @nikic could you kindly confirm this too? |
I don't really understand your question, but shufflevector semantics have been changed to use poison rather than undef, and we haven't really propagated this change to transforms yet. The UndefElts in SimplifyDemandedVectorElts need to be switched to PoisonElts. |
@nikic, sorry, my doubt was related to what extent we may allow miscompilation (if any?), if the concatenated shufflevector was being accessed out-of-bound (via OOB index in the mask). The UndefElts in @@ -1378,8 +1378,10 @@ Value *InstCombinerImpl::SimplifyDemandedVectorElts(Value *V,
if (!Elt) return nullptr;
Elts.push_back(Elt);
+#if 0
if (isa<UndefValue>(Elt)) // Already undef or poison.
UndefElts.setBit(i);
+#endif
} Seems to solve 1) and fixes https://github.com/llvm/llvm-project/blob/main/llvm/test/Transforms/InstCombine/insert-const-shuf.ll#L95-L104 test (which is currently wrong, https://alive2.llvm.org/ce/z/5TUcHp). |
Reproduction:
clang -O1
prints0
,clang -O2
prints a random number https://godbolt.org/z/jq5sfYGnb (on trunk 7003e25)Bisects down to
InstCombinePass
From Rust
The text was updated successfully, but these errors were encountered: