[PATCH] D71960: [Attributor] AAUndefinedBehavior: Use AAValueSimplify

[Stefanos Baziotis via Phabricator via llvm-commits]

baziotis added a comment.

In D71960#1873204 <https://reviews.llvm.org/D71960#1873204>, @jdoerfert wrote:

> In D71960#1872563 <https://reviews.llvm.org/D71960#1872563>, @baziotis wrote:
>
> > - Update commit message and test cases
> >
> >   in `ArgumentPromotion/2008-07-02-array-indexing.ll`, there was a weird `align` value that was only in the `CHECK` directives so it didn't seem it was coming from somewhere. I had to change it to make it pass. Please verify that it is not needed.
>
>
> The align was "derived" from a `null` pointer, which has every alignment. Since you made the UB go away by using an argument instead of loading from `null`, the alignment is no longer deduced. All as expected :)


Ok and so it put a random value, got it :)

> 
> 
> In D71960#1872571 <https://reviews.llvm.org/D71960#1872571>, @baziotis wrote:
> 
>> In D71960#1870868 <https://reviews.llvm.org/D71960#1870868>, @jdoerfert wrote:
>>
>> > 2. add a test that checks the functionality. the test changes in this patch "repair" existing tests to not expose unwanted UB but we want to have at least one test that exhibits UB and which gets exploited due to this patch.
>>
>>
>> Something more complicated than `load_null_propagated()` and / or `cond_br_on_undef*` functions (in `undefined_behavior.ll` testcase) ?
> 
> 
> Nothing complicated. Along those lines but as far as I can tell this patch does not include changes to these tests, correct?

Correct. TBH, it's been some time since I saw this that for example even I forgot that `load_null_propagated()` was not introduced in this patch (although it still puzzles me how it worked before this patch).
I'll add more tests.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71960/new/

https://reviews.llvm.org/D71960