[PATCH] D72291: Reimplement BoundaryAlign mechanism (mostly, but not quite NFC)

Philip Reames via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 6 10:16:36 PST 2020


reames created this revision.
reames added reviewers: skan, annita.zhang, craig.topper, MaskRay, jyknight, LuoYuanke.
Herald added subscribers: bollu, hiraditya, mcrosier.
Herald added a project: LLVM.

This patch is an implementation of a review suggestion I had made on the original review.  Essentially, rather than using up to two boundary align fragments to refine the region of interest, instead use a single one at the beginning of the region with an explicit end pointer.  I happen to think this is a simpler and more clear implementation, but it may be a matter of opinion.  I'm curious what others think.

This turned out to quite be NFC for an unexpected reason; we hadn't be flushing pending labels when starting a boundary align region.  As such labels between a pair of instructions would sometimes have nops inserted before the previous instruction and the resulting label placement.  As noted in the other negative tests, binding the label to the preceding instruction isn't *always* what you want, but it seems like a pretty reasonable default.

Since the patch was already NFC, I went ahead and reduced the maximum padding amount so that I could keep the size of a boundary align fragment the same on 64 bit platforms.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72291

Files:
  llvm/include/llvm/MC/MCFragment.h
  llvm/lib/MC/MCAssembler.cpp
  llvm/lib/MC/MCFragment.cpp
  llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
  llvm/test/MC/X86/align-branch-64-negative.s

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D72291.236404.patch
Type: text/x-patch
Size: 11924 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200106/862e0d24/attachment.bin>


More information about the llvm-commits mailing list