[PATCH] Add a fence elimination pass

hfinkel at anl.gov hfinkel at anl.gov
Wed Nov 12 08:03:02 PST 2014


>>! In D5758#28, @jfb wrote:
>> I agree it might be a good thing to run it anyway on all targets, but
> some tests (at least) on Power contain conditional jumps based on undef,
> and SimplifyCFG makes a complete mess of them.
> 
> Would it be worth fixing the tests and turning on SimplifyCFG in a first
> pass? Or as you suggested having a disable flag for this (icky...).

There might be no good solution here if we want to run SimplifyCFG generally -- these are bugpoint-generated tests, and thus have a lot of undefs, and will be sensitive to this kind of change. I think a flag is fine.

As noted below, however, a better solution is to have the new fences pass call the associated utility functions directly -- and as noted, this might require some amount of improvement to those utility functions to preserve BFI.

================
Comment at: lib/Target/PowerPC/PPCTargetMachine.cpp:184
@@ +183,3 @@
+    // FIXME: breaks a bunch of brittle tests
+    // addPass(createCFGSimplificationPass());
+  }
----------------
morisset wrote:
> hfinkel wrote:
> > If doing this is generally a good thing, then we should always do it (and on what target would that not be true?).
> > 
> > Otherwise, the CFG simplification pass is mostly a wrapper around the SimplifyCFG utility function, and perhaps it should just be called directly from the FencesPRE pass?
> > 
> I agree it might be a good thing to run it anyway on all targets, but some tests (at least) on Power contain conditional jumps based on undef, and SimplifyCFG makes a complete mess of them.
> 
> The cleanup is mostly because of requiring BreakCriticalEdges (that I have not found how to do on demand while preserving BlockFrequencyInfo yet), so calling it directly from FencesPRE would not solve the issue.
BreakCriticalEdges is a simple wrapper around the SplitCriticalEdge utility function. If we need to teach SplitCriticalEdge how to preserve BlockFrequencyInfo, then let's do that.

http://reviews.llvm.org/D5758






More information about the llvm-commits mailing list