User Details
- User Since
- Oct 3 2013, 11:31 AM (456 w, 6 d)
Sun, Jul 3
Sat, Jul 2
Please avoid using UndefValue::get whenever possible as we are trying to get rid of undef. Please use PoisonValue. Thank you!
Fri, Jul 1
Thu, Jun 30
Mon, Jun 27
Sun, Jun 26
Sat, Jun 25
Fri, Jun 24
Thu, Jun 23
One more comment: it would be great if you could update LangRef to mention the special case of fneg. Thanks!
Wed, Jun 22
Alive2 complains about one of the test cases. Could you please double check? Thanks
define float @test_float_fneg_pzero_f32_out() denormal-fp-math=ieee,ieee denormal-fp-math-f32=positive-zero,ieee { %result = fneg float 0.000000, exceptions=ignore ret float %result } => define float @test_float_fneg_pzero_f32_out() noread nowrite nofree willreturn denormal-fp-math=ieee,ieee denormal-fp-math-f32=positive-zero,ieee { ret float -0.000000 } Transformation doesn't verify!
Fri, Jun 17
Thu, Jun 16
LGTM
Could you please explain what's the bug and why is this the right fix?
Mon, Jun 13
There's no tradition in sharing tests between GVN & NewGVN. Are you going to implement the functionality in both new & old GVN?
If possible, please replace all uses of undef with poison. Thank you!
If possible, please replace all uses of undef with poison. Thank you!
Sun, Jun 12
Fri, Jun 10
Jun 6 2022
Jun 5 2022
Jun 4 2022
Jun 3 2022
That's a bug, yes. I won't comment on the fix as I don't know enough about SLP's code.
Jun 2 2022
Jun 1 2022
May 31 2022
LGTM, sounds like the right approach. Negative indexes are not that common anyway.
May 30 2022
May 29 2022
May 27 2022
May 26 2022
May 22 2022
May 20 2022
We believe that there's a bug when the 2nd argument is 0.
See here: https://gcc.godbolt.org/z/9735rYbqP
May 19 2022
May 10 2022
Somehow on my computer I don't get the same expected result for one of the tests:
$ cat aa.ll define i8 @sub_add_umin_commute_umin(i8 %x, i8 %y, i8 %z) { %a = add i8 %x, %y %m = call i8 @llvm.umin.i8(i8 %z, i8 %y) %s = sub i8 %a, %m ret i8 %s }
May 9 2022
May 4 2022
May 3 2022
May 2 2022
Is the second argument required to be a constant? If so, it would be great to document that. Thanks!
Apr 29 2022
Sounds great, thanks! We should avoid introducing ptr2int at all costs.
Well, I think this is ok. It's not regressing behavior vs undef store removal. So if this is important for Rust, let's maybe keep it,
Fingers crossed that the GSoC student will make good progress on the uninitialized memory thing.
Apr 26 2022
@regehr If you have time, it would be nice to fuzz LICM to check if this assertion can be triggered (after the patch lands).
It needs a function with the writeonly attribute + a pointer as input (or a global variable), a loop with a store to said pointer, a phi, and that's it (i.e., the test cases here as seed). Then some random stuff to make it crash.
Thanks!
LGTM. It's better than keeping freeze around. If we get concrete examples later of non-ideal "best" constants we can improve.
Apr 24 2022
I'm surprised the assertion is not an if. But if it doesn't trigger, then LGTM.
Apr 22 2022
LGTM, I think it reads well.
Apr 21 2022
Well, here we are discussing the semantics of LLVM IR. It's ok for the semantics of SDAG to be different, as long as it's a refinement of LLVM IR's.
Padding at IR level is not observable normally. So we can define it as poison.
Apr 20 2022
Not sure it's sufficient to restrict to the callers. Essentially we want to assert that these functions don't have state, so any read/writes they do is to locally allocated memory. No other function in the program can access that memory.
Apr 14 2022
sorry, never mind, Alive2's counterexample seems wrong. I'll dig up further.
Alive2 complains about this commit:
define <2 x i32> @modulo2_vec(<2 x i32> %x) { %0: %rem.i = srem <2 x i32> %x, { 2, 2 } %cmp.i = icmp slt <2 x i32> %rem.i, { 0, 0 } %add.i = select <2 x i1> %cmp.i, <2 x i32> { 2, 2 }, <2 x i32> { 0, 0 } %ret.i = add nsw <2 x i32> %add.i, %rem.i ret <2 x i32> %ret.i } => define <2 x i32> @modulo2_vec(<2 x i32> %x) { %0: %rem.i = srem <2 x i32> %x, { 2, 2 } %1 = and <2 x i32> %rem.i, { 2, 2 } %ret.i = add nsw <2 x i32> %1, %rem.i ret <2 x i32> %ret.i } Transformation doesn't verify! ERROR: Target is more poisonous than source
Apr 11 2022
I think the patch is not the full fix, as far as I understand. It's a good improvement, perf & correctness wise, though, but doesn't fix #51248.
Mar 19 2022
Mar 14 2022
I don't know how to frame this nicely, but I think it's important to set the expectations right: what kind of support can students expect from mentors? Mentors aren't their babysitter nor have the responsibility to fix the bugs for them. Nor fix compilation errors. I would like to have this written down given some bad experiences in the past.
Mentor's time is expensive and scarce, so use it wisely. The mentor is there to guide only, not to teach how to code.
Mar 10 2022
Mar 8 2022
For this transform only -- because we are guaranteed to repeat the values in each fcmp -- I was expecting that we could 'or' the relevant flags. But there's a surprising corner case with true and false fcmp predicates according to Alive2:
https://alive2.llvm.org/ce/z/Nffn3LThe behavior -- blocking poison via predicate? -- does not seem to be documented in the LangRef.
Mar 6 2022
Let me come back the usefulness question: which frontend can mark loops as finite but can't mark functions as willreturn?
This concept of sub-objects also shows up in the 'restrict' stuff. The difference is that with 'restrict' the aliasing constraints are dynamic: the promise is that e.g. two (dynamic) accesses in the original program don't alias. Here the constrains are static.