[PATCH] D18625: [Speculation] Add a SpeculativeExecution mode where the pass does nothing unless TTI::hasBranchDivergence() is true.

[Chandler Carruth via llvm-commits]

chandlerc accepted this revision.
chandlerc added a comment.
This revision is now accepted and ready to land.

Just some minor nits and tweaks to the testing below. Thanks. LGTM, feel free to submit with this stuff addressed, or ask for clarifications if any of it doesn't make sense.


================
Comment at: lib/Transforms/Scalar/SpeculativeExecution.cpp:106-108
@@ -91,1 +105,5 @@
+   static char ID;
+   explicit SpeculativeExecution(bool OnlyIfDivergentArch = false)
+       : FunctionPass(ID), OnlyIfDivergentArch(OnlyIfDivergentArch ||
+                                               SpecExecOnlyIfDivergentTarget) {}
 
----------------
I would use either ...Arch or ...Target consistently.

Does it make sense to make the flag be the default argument rather than an ||? I don't feel strongly either way.

================
Comment at: test/Transforms/SpeculativeExecution/divergent-target.ll:7
@@ +6,3 @@
+
+target triple = "nvptx-nvidia-cuda"
+
----------------
Maybe put this in '-mtriple' arguments to the two target specific ones? That way the "always" mode isn't tied to this target?

It would also make the '-march' above more clean as it could be a parallel '-mtriple=x86_64-...'

================
Comment at: test/Transforms/SpeculativeExecution/divergent-target.ll:11
@@ +10,3 @@
+define void @f() {
+; CHECK-LABEL: @ifThen(
+; ON: %x = add i32 2, 3
----------------
This looks stale?


http://reviews.llvm.org/D18625