[PATCH] D82703: [InstCombine] convert assumes to operand bundles

[Tyker via Phabricator via llvm-commits]

Tyker added inline comments.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:756
+      return;
+  }
+
----------------
jdoerfert wrote:
> Why can we allow ephemeral here without potentially throwing away an assume that was used to argue it itself is useless? Should we not do the `AllowEphemeral` stuff for now?
i fixed the issue AllowEphemeral was "solving" by other means.


================
Comment at: llvm/test/Analysis/ValueTracking/assume.ll:75
 ;
-  call void @llvm.assume(i1 true) ["dereferenceable"(i32* %0, i32 4)]
+  call void @llvm.assume(i1 true) ["dereferenceable"(i32* %0, i32 4), "align"(i32* %0, i32 8)]
   br i1 %cond, label %A, label %B
----------------
jdoerfert wrote:
> How do we get an alignment of 8 here?
this was just an addition arbitrary test. left the old test as is a added a new.


================
Comment at: llvm/test/Transforms/InstCombine/assume.ll:710
+  ret void
+}
+
----------------
Tyker wrote:
> jdoerfert wrote:
> > Also unclear if this is better than the GEP. I guess the case with an offset of 4 is clear but this one I don't know.
> this is one of the default canonicalization done when building assumes maybe we should keep both.
> the idea behind it is to make the knowledge usable by all derived pointers and also minimize the number of assumes but i agree it is not clear its better.
> 
I think we should try to avoid having any extra instructions that are only used by an assume because it is part of the issue with the boolean assume model.

With the current representation of RetainedKnowledge there is no way to represent offsets.
this restriction could be lifetd by refactoring and is not the only limitation of RetainedKnowledge that could be removed.
when forming a RetainedKnowledge of a "align"(i8* %3, i64 16, i64 4) the RetainedKnowledge will conservativly have the value 4.

since canonicalization is done on RetainedKnowledge cannonicalization can't interact with offsets and stays conservative.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82703