[PATCH] D134732: fix vectorization of library calls

[Tim Schmielau via Phabricator via llvm-commits]

tim.schmielau created this revision.
Herald added a subscriber: hiraditya.
Herald added a project: All.
tim.schmielau requested review of this revision.
Herald added subscribers: llvm-commits, pcwang-thead.
Herald added a project: LLVM.

Vectorization of library calls was unnecessarily fragile as
LoopAccessAnalysis would only consult the hardcoded list of
vectorizable functions for (call) instructions marked as potentially
reading from memory. However, few of the library functions in
llvm/include/llvm/Analysis/VecFuncs.def actually do that.

Thus commit ea75be3d9df448b6abafaf752a8141764d93ca33 <https://reviews.llvm.org/rGea75be3d9df448b6abafaf752a8141764d93ca33> and its
recommit 5f0a349738304caf5f8083166f785a91048f574d <https://reviews.llvm.org/rG5f0a349738304caf5f8083166f785a91048f574d> broke library
call vectorization by correctly declaring many library functions
as write only.

Apply the obvious fix of considering any call for vectorization,
not just those to functions that may read from memory.

Also change the libm-vector-calls tests so they would have
caught the issue.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134732

Files:
  llvm/lib/Analysis/LoopAccessAnalysis.cpp
  llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls-VF2-VF8.ll
  llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls.ll


Index: llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls.ll
===================================================================
--- llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls.ll
+++ llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls.ll
@@ -356,7 +356,8 @@
 !132 = !{!"llvm.loop.vectorize.width", i32 4}
 !133 = !{!"llvm.loop.vectorize.enable", i1 true}
 
-attributes #0 = { nounwind readnone }
+; functions are in fact "readnone" but clang only emits the weaker "writeonly" as other math functions may write errno.
+attributes #0 = { nounwind writeonly }
 
 declare double @sin(double) #0
 declare float @sinf(float) #0
Index: llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls-VF2-VF8.ll
===================================================================
--- llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls-VF2-VF8.ll
+++ llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls-VF2-VF8.ll
@@ -356,7 +356,8 @@
 !132 = !{!"llvm.loop.vectorize.width", i32 8}
 !133 = !{!"llvm.loop.vectorize.enable", i1 true}
 
-attributes #0 = { nounwind readnone }
+; functions are in fact "readnone" but clang only emits the weaker "writeonly" as other math functions may write errno.
+attributes #0 = { nounwind writeonly }
 
 declare double @sin(double) #0
 declare float @sinf(float) #0
Index: llvm/lib/Analysis/LoopAccessAnalysis.cpp
===================================================================
--- llvm/lib/Analysis/LoopAccessAnalysis.cpp
+++ llvm/lib/Analysis/LoopAccessAnalysis.cpp
@@ -2188,8 +2188,8 @@
     // Scan the BB and collect legal loads and stores. Also detect any
     // convergent instructions.
     for (Instruction &I : *BB) {
-      if (auto *Call = dyn_cast<CallBase>(&I)) {
-        if (Call->isConvergent())
+      if (auto *CB = dyn_cast<CallBase>(&I)) {
+        if (CB->isConvergent())
           HasConvergentOp = true;
       }
 
@@ -2204,14 +2204,10 @@
       if (HasComplexMemInst)
         continue;
 
-      // If this is a load, save it. If this instruction can read from memory
-      // but is not a load, then we quit. Notice that we don't handle function
-      // calls that read or write.
-      if (I.mayReadFromMemory()) {
+      if (auto *Call = dyn_cast<CallInst>(&I)) {
         // Many math library functions read the rounding mode. We will only
         // vectorize a loop if it contains known function calls that don't set
         // the flag. Therefore, it is safe to ignore this read from memory.
-        auto *Call = dyn_cast<CallInst>(&I);
         if (Call && getVectorIntrinsicIDForCall(Call, TLI))
           continue;
 
@@ -2220,7 +2216,12 @@
         if (Call && !Call->isNoBuiltin() && Call->getCalledFunction() &&
             !VFDatabase::getMappings(*Call).empty())
           continue;
+      }
 
+      // If this is a load, save it. If this instruction can read from memory
+      // but is not a load, then we quit. Notice that we don't handle function
+      // calls that read or write.
+      if (I.mayReadFromMemory()) {
         auto *Ld = dyn_cast<LoadInst>(&I);
         if (!Ld) {
           recordAnalysis("CantVectorizeInstruction", Ld)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D134732.463197.patch
Type: text/x-patch
Size: 3153 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220927/90870c51/attachment.bin>