[PATCH] D66967: [Attributor] ValueSimplify Abstract Attribute

[Hideto Ueno via Phabricator via llvm-commits]

uenoku marked an inline comment as done.
uenoku added inline comments.


================
Comment at: llvm/test/Transforms/FunctionAttrs/value-simplify.ll:117
+  ; CHECK: tail call void @use(i32 %select-not-same-undef)
+  tail call void @use(i32 %select-not-same-undef)
+
----------------
jdoerfert wrote:
> uenoku wrote:
> > jdoerfert wrote:
> > > This should arguably be
> > >   `; CHECK: tail call void @use(i32 %phi-not-same)`
> > > The pessimistic fixpoint should probably not revert to `nullptr` and cause a cascading failure but instead just go back to the original value.
> > >
> > > This should arguably be
> > > 
> > > `; CHECK: tail call void @use(i32 %phi-not-same)`
> > In genericTraversal, `%phi-not-same` is  decomposed to i32 0, 1 so I think it is not currect.
> > 
> > > The pessimistic fixpoint should probably not revert to nullptr and cause a cascading failure but instead just go back to the original value.
> > It looks good.
> so `%phi-not-same` is not simplified, right?
> But `%select-not-same-undef` is something or `undef` and then we should simplify it to "something". That would probably happen just fine if we do not use `nullptr` as pessimistic value but the original value itself. That is, the worst thing you can simplify an instruction to is itself.
> so %phi-not-same is not simplified, right?
Yes
> But %select-not-same-undef is something or undef and then we should simplify it to  "something". 
In the simplification of %select-not-same-undef, traversed values are `0, 1, undef` and we don't look at %phi-not-same variable itself.

Anyway, I changed to return original value in the pessimistic state and simplified value(always not equals to original value ) in the optimistic state.



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

https://reviews.llvm.org/D66967