[PATCH] Add a fence elimination pass

Robin Morisset morisset at google.com
Mon Oct 13 12:43:53 PDT 2014


Hi jfb, echristo, rengolin,

Following the plan described on LLVM-dev, this patch provides a first version
of a new fence-elimination pass. It has several known restrictions/weaknesses,
but I would like some feedback early, hence this review.

The main problem is that there is no easy way to only run the pass on a
function if AtomicExpand introduced fences in that function. This is made
worse by the requirement on BlockFrequencyInfo and BreakCriticalEdges of
this pass:
- There is a non-negligible cost incurred even on code with no atomics/fences
- Because BreakCriticalEdges changes control-flow, it breaks a test
  (and running SimplifyCFG afterwards which would be nice would break even more
  tests).
I can see several ways to fix this:
- Merge the pass in AtomicExpand. Beyond the ugliness, this would require a way
  of running BreakCriticalEdges and BlockFrequencyInfo from inside AtomicExpand
  which does not seem supported by the current PassManager infrastructure.
- Break critical edges lazily inside this pass. This would still incurs costs
  for BlockFrequencyInfo; worse it would require updating the block frequency
  as well, and would add some significant complexity to the pass.
- Have a flag for running this pass and set it to false for the tests this
  break. This is the easiest but would do nothing for the performance costs.
What would you suggest ?

Some other parts remain to be done, but can be added later, and I hope to first
fix the above issue. For example:
- the pass is only enabled for Power, and not yet for ARM
- the min-cut implementation is rather basic and not super optimized
- the pass does not make use of information passed by AtomicExpand to optimize
  fences more agressively (for example it cannot sink the fence out of a
  spinloop yet), details on how I plan to do that later are in the proposal I
  sent to LLVM-dev some time ago.

Some more questions for reviewers:
- should the min-cut implementation be cut in a separate file ?
- I left lots of DEBUG() statements, should they be removed ?

Thanks for taking the time of reading this !

http://reviews.llvm.org/D5758

Files:
  include/llvm/InitializePasses.h
  include/llvm/Transforms/Scalar.h
  lib/Target/PowerPC/PPCTargetMachine.cpp
  lib/Transforms/Scalar/CMakeLists.txt
  lib/Transforms/Scalar/FencesPRE.cpp
  lib/Transforms/Scalar/Scalar.cpp
  test/CodeGen/PowerPC/atomics-fences-pre.ll
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D5758.14813.patch
Type: text/x-patch
Size: 26793 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141013/0229c90a/attachment.bin>


More information about the llvm-commits mailing list