[PATCH] D85178: [Attributor} Check violation of returned position nonnull and noundef attribute in AAUndefinedBehavior

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

Cool :)

There is a typo in your subject line `[Attributor]`.



================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:2052
+          if (!ValueSimplifyAA.isKnown())
+            return true;
+
----------------
I don't think we need to simplify the value here. If it is not simplified, it should happen as part of AAReturned. I guess we can just not do it for now?


================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:2057
+          //   (1) Returned value is known to be dead (because it can be reduced
+          //       to undef).
+          //   (2) The value is known to be undef.
----------------
Reading this makes me question if it should ever happen. I hope AAReturnedValues will not look at dead returns so they should not be part of the RetInsts set and if that set is empty this should never be called.


================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:2074-2079
+          if (FoundUB) {
+            for (ReturnInst *RI : RetInsts) {
+              Instruction *I = static_cast<Instruction *>(RI);
+              if (!AssumedNoUBInsts.count(I) && !KnownUBInsts.count(I))
+                KnownUBInsts.insert(I);
+            }
----------------



================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:2097
+      A.checkForAllReturnedValuesAndReturnInsts(InspectReturnInstForUB, *this);
+
     if (NoUBPrevSize != AssumedNoUBInsts.size() ||
----------------
Note: When the other patch lands we should check if we want to ask the AANoUndef here.


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

https://reviews.llvm.org/D85178