[PATCH] D130241: [NFC][GVN] Put phi-translation of 'add' behind a switch

[Peter Waller via Phabricator via llvm-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf8919d2f7eba: [NFC][GVN] Put phi-translation of 'add' behind a switch (authored by peterwaller-arm).

Changed prior to commit:
  https://reviews.llvm.org/D130241?vs=446610&id=447212#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130241

Files:
  llvm/lib/Analysis/PHITransAddr.cpp
  llvm/test/Transforms/GVN/PRE/phi-translate-add.ll


Index: llvm/test/Transforms/GVN/PRE/phi-translate-add.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/GVN/PRE/phi-translate-add.ll
@@ -0,0 +1,44 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -gvn -gvn-add-phi-translation=true  -S < %s | FileCheck %s --check-prefix=ADD-TRANS-ON
+; RUN: opt -gvn -gvn-add-phi-translation=false -S < %s | FileCheck %s --check-prefix=ADD-TRANS-OFF
+
+; Test that phi translation is able to hoist a load whose address
+; depends on an add also being hoisted.
+define double @phi_translation_hoists_add(ptr %a, i64 %idx) {
+; ADD-TRANS-ON-LABEL: @phi_translation_hoists_add(
+; ADD-TRANS-ON-NEXT:  entry:
+; ADD-TRANS-ON-NEXT:    [[ADD_PHI_TRANS_INSERT:%.*]] = add nuw nsw i64 [[IDX:%.*]], 1
+; ADD-TRANS-ON-NEXT:    [[GEP_PHI_TRANS_INSERT:%.*]] = getelementptr inbounds double, ptr [[A:%.*]], i64 [[ADD_PHI_TRANS_INSERT]]
+; ADD-TRANS-ON-NEXT:    [[LOAD_PRE:%.*]] = load double, ptr [[GEP_PHI_TRANS_INSERT]], align 8
+; ADD-TRANS-ON-NEXT:    br label [[FOR_BODY:%.*]]
+; ADD-TRANS-ON:       for.body:
+; ADD-TRANS-ON-NEXT:    [[CMP:%.*]] = fcmp ole double [[LOAD_PRE]], 1.000000e+00
+; ADD-TRANS-ON-NEXT:    br i1 [[CMP]], label [[EXIT:%.*]], label [[FOR_BODY]]
+; ADD-TRANS-ON:       exit:
+; ADD-TRANS-ON-NEXT:    ret double [[LOAD_PRE]]
+;
+; ADD-TRANS-OFF-LABEL: @phi_translation_hoists_add(
+; ADD-TRANS-OFF-NEXT:  entry:
+; ADD-TRANS-OFF-NEXT:    br label [[FOR_BODY:%.*]]
+; ADD-TRANS-OFF:       for.body:
+; ADD-TRANS-OFF-NEXT:    [[ADD:%.*]] = add nuw nsw i64 [[IDX:%.*]], 1
+; ADD-TRANS-OFF-NEXT:    [[GEP:%.*]] = getelementptr inbounds double, ptr [[A:%.*]], i64 [[ADD]]
+; ADD-TRANS-OFF-NEXT:    [[LOAD:%.*]] = load double, ptr [[GEP]], align 8
+; ADD-TRANS-OFF-NEXT:    [[CMP:%.*]] = fcmp ole double [[LOAD]], 1.000000e+00
+; ADD-TRANS-OFF-NEXT:    br i1 [[CMP]], label [[EXIT:%.*]], label [[FOR_BODY]]
+; ADD-TRANS-OFF:       exit:
+; ADD-TRANS-OFF-NEXT:    ret double [[LOAD]]
+;
+entry:
+  br label %for.body
+
+for.body:                                         ; preds = %for.body, %entry
+  %add = add nuw nsw i64 %idx, 1
+  %gep = getelementptr inbounds double, ptr %a, i64 %add
+  %load = load double, ptr %gep
+  %cmp = fcmp ole double %load, 1.000000e+00
+  br i1 %cmp, label %exit, label %for.body
+
+exit:
+  ret double %load
+}
Index: llvm/lib/Analysis/PHITransAddr.cpp
===================================================================
--- llvm/lib/Analysis/PHITransAddr.cpp
+++ llvm/lib/Analysis/PHITransAddr.cpp
@@ -21,6 +21,10 @@
 #include "llvm/Support/raw_ostream.h"
 using namespace llvm;
 
+static cl::opt<bool> EnableAddPhiTranslation(
+    "gvn-add-phi-translation", cl::init(false), cl::Hidden,
+    cl::desc("Enable phi-translation of add instructions"));
+
 static bool CanPHITrans(Instruction *Inst) {
   if (isa<PHINode>(Inst) ||
       isa<GetElementPtrInst>(Inst))
@@ -410,14 +414,14 @@
     return Result;
   }
 
-#if 0
-  // FIXME: This code works, but it is unclear that we actually want to insert
-  // a big chain of computation in order to make a value available in a block.
-  // This needs to be evaluated carefully to consider its cost trade offs.
-
   // Handle add with a constant RHS.
-  if (Inst->getOpcode() == Instruction::Add &&
+  if (EnableAddPhiTranslation && Inst->getOpcode() == Instruction::Add &&
       isa<ConstantInt>(Inst->getOperand(1))) {
+
+    // FIXME: This code works, but it is unclear that we actually want to insert
+    // a big chain of computation in order to make a value available in a block.
+    // This needs to be evaluated carefully to consider its cost trade offs.
+
     // PHI translate the LHS.
     Value *OpVal = InsertPHITranslatedSubExpr(Inst->getOperand(0),
                                               CurBB, PredBB, DT, NewInsts);
@@ -431,7 +435,6 @@
     NewInsts.push_back(Res);
     return Res;
   }
-#endif
 
   return nullptr;
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D130241.447212.patch
Type: text/x-patch
Size: 3972 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220725/12096151/attachment.bin>