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

[Chandler Carruth via llvm-commits]

chandlerc added a comment.

In http://reviews.llvm.org/D18625#398483, @jlebar wrote:

> > Please add a pass-local flag that allows you to test the pass in both modes, and some test cases exercising the behavior here.
>
>
> Chandler, how would you feel if I added a test to http://reviews.llvm.org/D18626 instead?  (The test would check that we do the speculative execution only on the appropriate targets.)


You should also probably do some testing there, but...

> Adding a flag to force speculative execution on (or off?) doesn't seem to test what we're actually after, namely that we do SpeculativeExecution iff the target has divergent branches.


I'm not suggesting you force speculation on or off here. I'm suggesting you toggle the two modes described in the comments: "always" vs "only if divergent arch". Essentially, a flag that simulates the two public functions you can call to construct the pass.

You can use a target that will trivially never have speculation enabled to ensure you can observe this.

I'm essentially saying you should test two different things:

1. The pass has an "always" mode that doesn't care about the target. That should involve a flag here.

2. Targets can differentially control this when not in the "always" mode. That can involve having a flag for a single target that swings it both ways, or a flag that swings all targets, or just comparing two known targets. However, the last of these makes it annoying because we have to disable the test if *either* of the targets are missing.


http://reviews.llvm.org/D18625