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

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

Now all that is left is a test file with positive and negative tests so we can see it works and not accidentally break it in the future.

In D71435#1787126 <https://reviews.llvm.org/D71435#1787126>, @baziotis wrote:

> - `manifest` that changes to uncreachable live UB loads.
> - Set both for UB loads and non-UB loads.
> - Stats that use UB loads.
> - Method that checks if a specific (load) instruction is considered UB.


The method needs to be `isAssumedToCauseUB` not `isKnownToCauseUB` because we don't know until a fixpoint is reached.
Also remove the ones that do not take an instruction for now. They are confusing and not needed until they actually return the proper value, I mean true if the function *always* exhibits UB.

> Note: Clang format did its job a little bit too well and replaced 2 parts that are not related to this patch. Should I remove them?

Two options: 1) Remove them from the diff. 2) Commit them first.

Since I will probably have to actually push the diff (as you do not have push access) I will do a clang format of the file first.
It also helps to only use the clang format diff script during development. Though, the Attributor files should always be formatted which is why option 2) above is a proper way to resolve this.



================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:1998
+    AAUndefinedBehavior::initialize(A);
+  }
+
----------------
Nit: This is the same as not overriding the method in the first place.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2032
+      return true;
+    };
+    A.checkForAllInstructions(CannotCauseUB, *this, {Instruction::Load});
----------------
Nit: Add a TODO above explaining that we should not only look at loads and also expand it to more than *constant* null values eventually.
Nit: Empty lines, e.g., before comments, help (me) to read code. 


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2041
+    return (UBLoads.find(I) != UBLoads.end());
+  }
+
----------------
Nit: `UBLoads.count(I)` is shorter, also in some other places.


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

https://reviews.llvm.org/D71435