User Details
- User Since
- Oct 3 2013, 11:31 AM (495 w, 1 d)
Sat, Mar 18
Mon, Mar 13
LGTM
Fri, Mar 10
Thu, Mar 9
tests failing
tests failing
tests failing
tests are failing. please run all the tests before submitting patches.
Sun, Mar 5
Mar 2 2023
Feb 22 2023
Please use poison values as place holders instead of undef values as we're trying to get rid of Undef. Thank you!
Feb 15 2023
This is great, thank you!
Feb 12 2023
Feb 10 2023
Feb 5 2023
Jan 20 2023
Jan 19 2023
LGTM, except for the align metadata.
Jan 16 2023
The issue is the function call, not the stores.
Stores are valid as long as they are to local memory (stack or allocated by the function).
Jan 12 2023
Three failures:
- Transforms/InstCombine/loadstore-metadata.ll
- Transforms/Mem2Reg/preserve-nonnull-load-metadata.ll
- Transforms/SROA/preserve-nonnull.ll
Jan 11 2023
Sounds good, thanks!
I implemented this in Alive2 and I got zero failures in LLVM's test suite. Either it doesn't have many !ranges or we are good.
LGTM
Jan 9 2023
I must say that this patch is a bit annoying :)
We are trying to remove undef, and you're adding yet another use.
It's true we can't use poison here. The alternative in the no-undef world is 'freeze poison'. Or just leave the zero there.
Is this change important? If not, I would prefer to not do it, as we'll have to revert it when removing undef (hopefully later this year).
Thanks!
Jan 4 2023
LGTM, thanks!
Jan 3 2023
LGTM!
Dec 23 2022
Dec 22 2022
LGTM, thank you!
Dec 20 2022
Dec 16 2022
Dec 14 2022
Alive complains about this test case:
define <8 x i16> @insert_10_v8i16(i32 %x, <8 x i16> %v) { %0: %hi32 = lshr i32 %x, 16 %hi16 = trunc i32 %hi32 to i16 %lo16 = trunc i32 %x to i16 %ins0 = insertelement <8 x i16> %v, i16 %lo16, i64 1 %ins1 = insertelement <8 x i16> %ins0, i16 %hi16, i64 0 ret <8 x i16> %ins1 } => define <8 x i16> @insert_10_v8i16(i32 %x, <8 x i16> %v) { %0: %1 = bitcast <8 x i16> %v to <4 x i32> %2 = insertelement <4 x i32> %1, i32 %x, i64 0 %ins1 = bitcast <4 x i32> %2 to <8 x i16> ret <8 x i16> %ins1 } Transformation doesn't verify! (unsound) ERROR: Target is more poisonous than source
FWIW, I've discovered today that GVN does a similar optimization (but without the freeze..).
See here (scroll to the bottom): https://web.ist.utl.pt/nuno.lopes/alive2/index.php?hash=aed14c64378404c9&test=Transforms%2FPhaseOrdering%2FX86%2Fvec-load-combine.ll
Dec 11 2022
LGTM
LGTM
Dec 6 2022
Dec 5 2022
LGTM
Thanks.
unspecified is not a term used in LLVM IR semantics. I would write, for example, "a non-deterministic value (equivalent to freeze poison)".
LGTM
Dec 4 2022
Dec 1 2022
Nov 30 2022
Nov 24 2022
Nov 22 2022
Manuel: there are quite a few test cases failing in the build bots: https://lab.llvm.org/buildbot/#/builders/16/builds/38352
Nov 21 2022
LGTM
Nov 17 2022
Nov 16 2022
Nov 2 2022
Nov 1 2022
Oct 30 2022
It seems this patch contradicts the proposed LangRef patch. The LangRef patch says that you can't assume that a volatile operation yields the same result in different spaces.
And I think that's a fair condition. Otherwise we need to document that constraint for hardware vendors and change LangRef. Pick one of the patches basically :)
I like this latest writing. But please wait for another reviewer to make sure I didn't miss smth.
Oct 29 2022
Oct 28 2022
Oct 26 2022
Oct 24 2022
Oct 21 2022
Oct 20 2022
Please avoid using UndefValue::get as much as possible as we are trying to get rid of undef. Please use PoisonValue whenever possible.
Thank you!
Oct 19 2022
However, multiple loads of the same memory must be equal
Oct 18 2022
Oct 17 2022
Oct 11 2022
Oct 8 2022
I would also prefer to not go this route.
Alternatives include using predicated builtins, stick to pre-headers, or introduce a new intrinsic that yields poison on division by zero and/or INT_MAX/-1.