[PATCH] D86737: [Attributor] Fix AANoUndef identification

Sat Aug 29 03:01:50 PDT 2020

okura added inline comments.

================
Comment at: llvm/test/Transforms/Attributor/IPConstantProp/PR16052.ll:20
 ; IS__CGSCC____-NEXT:  entry:
-; IS__CGSCC____-NEXT:    [[CALL2:%.*]] = call i64 @fn1(i64 undef)
+; IS__CGSCC____-NEXT:    [[CALL2:%.*]] = call i64 @fn1(i64 noundef undef)
 ; IS__CGSCC____-NEXT:    ret i64 [[CALL2]]
----------------
okura wrote:
> jdoerfert wrote:
> > Hm,... I think this exposes a problem. Could you take a look why we get a noundef here?
> I overlooked this problem, thanks.
> In this case, I expected that we indicate a pessimistic fixpoint for `%div = sdiv ...` in `genericValueTraversal` because we cannot strip value even once.
> A simplified value for `%div` does not have value (i.e. `llvm::None`) because the assumed range is empty. (we can intentionally reduce undef to 0 there.)
> Therefore, a callback is not called in `genericValueTraversal` and it returns `true`. That's why we got a noundef here.
> We can avoid this by changing `manifest` not to manifest for such positions.
I made a new patch for this. D86835
However, I'm worried that a similar problem will emerge again when another AA has the possibility to change values to undef.
I think implementing a new interface of Attributor to query if a position will be changed to undef is one of the possible solutions. But it would not be trivial change because we should manage the order of manifestation. (i.e. we should process noundef manifestation at last.)
How do you think about this?

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

https://reviews.llvm.org/D86737