[PATCH] D107976: [SelectionDAGBuilder] Compute and cache PreferredExtendType on demand.

[Craig Topper via Phabricator via llvm-commits]

craig.topper created this revision.
craig.topper added reviewers: efriedma, arsenm, RKSimon, spatel.
Herald added subscribers: luismarques, s.egerton, PkmX, simoncook, hiraditya, kristof.beyls.
craig.topper requested review of this revision.
Herald added a subscriber: wdng.
Herald added a project: LLVM.

Previously we pre-calculated this and cached it for every
instruction in the function. Most of the calculated results will
never be used. So instead calculate it only on the first use, and
then cache it.

The cache was originally added to fix a compile time issue which
caused r216066 to be reverted.

This change exposed that we weren't pre-computing the Value for
Arguments. I've explicitly disabled that for now as it seemed to
regress some tests on AArch64 which has sext built into its compare
instructions.

Spotted while investigating how to improve heuristics to work better
with RISCV preferring sign extend for unsigned compares for i32 on RV64.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107976

Files:
  llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp


Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
===================================================================
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -9857,6 +9857,34 @@
   llvm_unreachable("LowerOperation not implemented for this target!");
 }
 
+static ISD::NodeType getPreferredExtendForValue(const Value *V) {
+  ISD::NodeType ExtendKind = ISD::ANY_EXTEND;
+
+  // TODO: Skip Arguments to match behavior when this was calculated only for
+  // instructions.
+  if (isa<Argument>(V))
+    return ExtendKind;
+
+  // For the users of the source value being used for compare instruction, if
+  // the number of signed predicate is greater than unsigned predicate, we
+  // prefer to use SIGN_EXTEND.
+  //
+  // With this optimization, we would be able to reduce some redundant sign or
+  // zero extension instruction, and eventually more machine CSE opportunities
+  // can be exposed.
+  unsigned NumOfSigned = 0, NumOfUnsigned = 0;
+  for (const User *U : V->users()) {
+    if (const auto *CI = dyn_cast<CmpInst>(U)) {
+      NumOfSigned += CI->isSigned();
+      NumOfUnsigned += CI->isUnsigned();
+    }
+  }
+  if (NumOfSigned > NumOfUnsigned)
+    ExtendKind = ISD::SIGN_EXTEND;
+
+  return ExtendKind;
+}
+
 void
 SelectionDAGBuilder::CopyValueToVirtualRegister(const Value *V, unsigned Reg) {
   SDValue Op = getNonRegisterValue(V);
@@ -9873,10 +9901,12 @@
                    None); // This is not an ABI copy.
   SDValue Chain = DAG.getEntryNode();
 
-  ISD::NodeType ExtendType = ISD::ANY_EXTEND;
-  auto PreferredExtendIt = FuncInfo.PreferredExtendType.find(V);
-  if (PreferredExtendIt != FuncInfo.PreferredExtendType.end())
-    ExtendType = PreferredExtendIt->second;
+  // Look up the preferred extend kind if we've already computed it. Otherwise,
+  // compute it and cache it.
+  ISD::NodeType &ExtendType = FuncInfo.PreferredExtendType[V];
+  if (!ExtendType)
+    ExtendType = getPreferredExtendForValue(V);
+
   RFV.getCopyToRegs(Op, DAG, getCurSDLoc(), Chain, nullptr, V, ExtendType);
   PendingExports.push_back(Chain);
 }
Index: llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
===================================================================
--- llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
+++ llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
@@ -57,28 +57,6 @@
   return false;
 }
 
-static ISD::NodeType getPreferredExtendForValue(const Value *V) {
-  // For the users of the source value being used for compare instruction, if
-  // the number of signed predicate is greater than unsigned predicate, we
-  // prefer to use SIGN_EXTEND.
-  //
-  // With this optimization, we would be able to reduce some redundant sign or
-  // zero extension instruction, and eventually more machine CSE opportunities
-  // can be exposed.
-  ISD::NodeType ExtendKind = ISD::ANY_EXTEND;
-  unsigned NumOfSigned = 0, NumOfUnsigned = 0;
-  for (const User *U : V->users()) {
-    if (const auto *CI = dyn_cast<CmpInst>(U)) {
-      NumOfSigned += CI->isSigned();
-      NumOfUnsigned += CI->isUnsigned();
-    }
-  }
-  if (NumOfSigned > NumOfUnsigned)
-    ExtendKind = ISD::SIGN_EXTEND;
-
-  return ExtendKind;
-}
-
 void FunctionLoweringInfo::set(const Function &fn, MachineFunction &mf,
                                SelectionDAG *DAG) {
   Fn = &fn;
@@ -233,9 +211,6 @@
       if (isUsedOutsideOfDefiningBlock(&I))
         if (!isa<AllocaInst>(I) || !StaticAllocaMap.count(cast<AllocaInst>(&I)))
           InitializeRegForValue(&I);
-
-      // Decide the preferred extend type for a value.
-      PreferredExtendType[&I] = getPreferredExtendForValue(&I);
     }
   }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D107976.366041.patch
Type: text/x-patch
Size: 3717 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210812/a542c7cb/attachment.bin>