[PATCH] D95539: [LV] Add analysis remark for mixed precision conversions

[Florian Hahn via Phabricator via llvm-commits]

fhahn requested changes to this revision.
fhahn added a comment.
This revision now requires changes to proceed.

Please add test cases for the change.



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9290
+
+        // Traverse the uses of this floating point store inside the loop.
+        DenseMap<Instruction *, bool> Visited;
----------------
This potentially is a bit confusing, because a store has no uses (it's of type `void`). We are traversing the operands upwards.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9292
+        DenseMap<Instruction *, bool> Visited;
+        SmallVector<Instruction *, 8> Worklist;
+        Worklist.push_back(S);
----------------
Can we collect all FP stores in a first pass, and then do the wordlist traversal for the all fp stores together afterwards? This way, we won't get duplicated remarks, if the same value is stored multiple times.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9636
 
+    // runtime checks that get used.
     // Report the vectorization decision.
----------------
Unrelated change?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9647
+    auto &Ctx = L->getHeader()->getModule()->getContext();
+    if (Ctx.getDiagHandlerPtr()->isAnyRemarkEnabled(LV_NAME))
+      checkMixedPrecision(L, ORE);
----------------
I think usually `ORE->allowExtraAnalysis(LV_NAME);` is used to check if extra analysis should be done for remarks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95539