[PATCH] D71435: [WIP] [Attributor] Function level undefined behavior attribute

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/include/llvm/Transforms/IPO/Attributor.h:1699
+  /// Return true if "undefined behavior" is known.
+  bool isKnownToCauseUB() const { return getKnown(); }
+
----------------
baziotis wrote:
> jdoerfert wrote:
> > Since we start with a function version only, we probably need to add an `llvm::Instruction` operand here.
> > While it makes sense if *every* execution of a function causes UB, the initial implementation will
> > answer the questions for a single (load) instruction.
> You mean to add a (method) overload for `isKnownToCauseUB`, maybe pure `virtual` that is implemented in `AAUndefinedBehaviorImpl`? It can check if it is a `Load` and if so, check if it is in `UBLoads`.
Exactly. For now that is probably the only two methods we need. I mean we do not actually use the boolean state right now that is returned in the current `isAssumedToCauseUB`.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2040
+  }
+};
+
----------------
baziotis wrote:
> jdoerfert wrote:
> > Let's also add a manifest method to replace the loads that are still considered UB with an undef (for now). That should allow us to test this.
> The whole instruction? Like replace `%a = load i32, i32* null` with `undef`. I don't think this can happen. Maybe I misunderstood and you meant replace it with this: `%a = load i32, i32* undef` (i.e. replace the pointer operand value).
Your first interpretation was what I meant. Replace the load (instruction) with undef. We actually want to be more aggressive but we will need that step either way.


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

https://reviews.llvm.org/D71435