[PATCH] D83752: [ValueTracking] Let isGuaranteedNotToBeUndefOrPoison consider noundef and more operations

Juneyoung Lee via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 15 17:32:04 PDT 2020


aqjune marked an inline comment as done.
aqjune added a comment.

Will split this patch into (1) noundef patch (2) use operator patch



================
Comment at: llvm/include/llvm/Analysis/ValueTracking.h:610
+  /// If IgnorePadding is true, V's paddings are not inspected. If false,
+  /// the paddings should be frozen as well.
   bool isGuaranteedNotToBeUndefOrPoison(const Value *V,
----------------
nikic wrote:
> aqjune wrote:
> > nikic wrote:
> > > Why do we need this parameter? Padding only matters for the stored representation of a struct. For SSA struct values, the padding cannot be accessed.
> > It is because freeze can take an aggregate value.
> > I found that its behavior w.r.t aggregate values is not specified in LangRef, but I think it is good to make it freeze the paddings too because it guarantees that `v = freeze(load p); store v, q` will never have undef bits or poison in memory.
> > If this sounds good, I'll make a patch for this.
> I don't think something like this can work under current IR semantics. Even if we take a simpler example like `%v = freeze(load i1 %p); store %v, %q`, the freeze will only freeze the i1 value, it does not do anything to the remaining 7 bits that are present in the store representation. It does make sure that a subsequent load i1 will not see undef/poison, but not that a load i8 won't. For the aggregate case, similarly, it does make sure that loading the same aggregate type will not have undef/poison (because padding does not matter for the SSA value), but does not make sure that loading an appropriately sized iN at the same location will not have undef/poison.
Okay, that makes sense. There is no way to remove the 7-bit undef value when storing i1 value.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83752





More information about the llvm-commits mailing list