[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