Page MenuHomePhabricator

[llvm-readelf, propeller] Add fallthrough bit to basic block metadata in BB-Address-Map section.
ClosedPublic

Authored by rahmanl on Feb 17 2021, 4:56 PM.

Details

Summary

This patch adds a fallthrough bit to basic block metadata, indicating whether the basic block can fallthrough without taking any branches. The bit will help us avoid an intel LBR bug which results in occasional duplicate entries at the beginning of the LBR stack.

This patch uses MachineBasicBlock::canFallThrough() to set the bit. This is not a const method because it eventually calls TargetInstrInfo::analyzeBranch, but it calls this function with the default AllowModify=false. So we can either make the argument to the getBBAddrMapMetadata non-const, or we can use const_cast when calling canFallThrough. I decide to go with the latter since this is purely due to legacy code, and in general we should not allow the BasicBlock to be mutable during getBBAddrMapMetadata.

Diff Detail

Event Timeline

rahmanl created this revision.Feb 17 2021, 4:56 PM
rahmanl updated this revision to Diff 329808.Mar 10 2021, 5:31 PM

Rebase after landing c245c21c436b.

rahmanl retitled this revision from Add fallthrough bit to BasicBlock Metadata. to [llvm-readelf, propeller] Add fallthrough bit to basic block metadata in BB-Address-Map section..Mar 10 2021, 5:55 PM
rahmanl edited the summary of this revision. (Show Details)
rahmanl added reviewers: shenhan, MaskRay.
rahmanl removed a reviewer: MaskRay.
rahmanl added a subscriber: MaskRay.
rahmanl added subscribers: tmsriram, snehasish.
rahmanl updated this revision to Diff 329811.Mar 10 2021, 5:58 PM

Modify comment for AsmPrinter::getBBAddrMapMetadata.

rahmanl published this revision for review.Mar 10 2021, 5:58 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 10 2021, 5:58 PM

LGTM

Thanks for making the change.

llvm-readobj aspect looks fine.

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
1081

(whilst you're changing this line)

rahmanl updated this revision to Diff 330893.Tue, Mar 16, 12:46 AM
  • Fix typo.
  • Rebase.
rahmanl marked an inline comment as done.

Thanks for the quick review.
@tmsriram Would you please review the AsmPrinter.cpp change?

tmsriram accepted this revision.Mon, Mar 22, 3:37 PM

LGTM and is a simple change to capture the fall through bit.

This revision is now accepted and ready to land.Mon, Mar 22, 3:37 PM