[PATCH] D109450: [IRSim][IROutliner] Allowing Intrinsic Calls to be Used in Similarity Matching and Outlined Regions

[Jessica Paquette via Phabricator via llvm-commits]

paquette added inline comments.


================
Comment at: llvm/include/llvm/Analysis/IRSimilarityIdentifier.h:226
           llvm::hash_combine_range(OperTypes.begin(), OperTypes.end()));
-    else if (CallInst *CI = dyn_cast<CallInst>(ID.Inst))
+    else if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(ID.Inst)) {
+      // To hash intrinsics, we use the opcode, and types like the other
----------------
this always returns, right?

why not just make it an `if`?


================
Comment at: llvm/include/llvm/Analysis/IRSimilarityIdentifier.h:246
+          llvm::hash_combine_range(OperTypes.begin(), OperTypes.end()));
+    } else if (CallInst *CI = dyn_cast<CallInst>(ID.Inst))
       return llvm::hash_combine(
----------------
this can also just be an `if`?


================
Comment at: llvm/include/llvm/Analysis/IRSimilarityIdentifier.h:506
+      // an outlined function. Also, assume-like intrinsics could be removed,
+      // removing arguments, and make it unclear how to handle these cases.
+      if (II.isLifetimeStartOrEnd() || II.isAssumeLikeIntrinsic())
----------------
can you expand on the last sentence here?


================
Comment at: llvm/lib/Analysis/IRSimilarityIdentifier.cpp:34
+    DisableIntrinsics("no-ir-sim-intrinsics", cl::init(false), cl::ReallyHidden,
+                      cl::desc("disable similarity matching, and outlining, "
+                               "of intrinsic instructions."));
----------------
change passive voice?

"Don't match or outline intrinsics."


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

https://reviews.llvm.org/D109450