Page MenuHomePhabricator

Add "llvm_unreachable" for function RegisterBankInfo::getRegBank
AbandonedPublic

Authored by skan on May 16 2019, 6:03 AM.

Details

Summary

RegClassOrBank is an object of RegClassOrRegBank, which is defined as

using llvm::RegClassOrRegBank = typedef PointerUnion<const TargetRegisterClass *, const RegisterBank *>
}

so control flow can not get here. Use ""llvm_unreachable" here to avoid "null pointer" confusion.

Diff Detail

Event Timeline

skan created this revision.May 16 2019, 6:03 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 16 2019, 6:03 AM
skan updated this revision to Diff 200203.May 19 2019, 9:31 PM

add "const" to pointer type description.

arsenm accepted this revision.Mon, May 20, 3:52 AM
This revision is now accepted and ready to land.Mon, May 20, 3:52 AM
dsanders added inline comments.Mon, May 20, 10:00 AM
lib/CodeGen/GlobalISel/RegisterBankInfo.cpp
96 ↗(On Diff #200203)

There's a typo here ('Reigister')

skan updated this revision to Diff 200381.Mon, May 20, 6:11 PM

fix the typo ("Register")

skan marked an inline comment as done.Mon, May 20, 6:11 PM
skan added inline comments.
lib/CodeGen/GlobalISel/RegisterBankInfo.cpp
96 ↗(On Diff #200203)

fixed in the updated patch

skan added a subscriber: pengfei.Tue, May 28, 5:51 PM
This revision was automatically updated to reflect the committed changes.
qcolombet reopened this revision.Thu, May 30, 9:30 AM
qcolombet added inline comments.
llvm/lib/CodeGen/GlobalISel/RegisterBankInfo.cpp
96

Sorry for the late reply but that doesn't seem right.

Yes, RegClassOrRegBank is of one of the two classes, but it can still be nullptr, e.g., before regbankselect. Therefore this case seems reachable to me.

What am I missing?

This revision is now accepted and ready to land.Thu, May 30, 9:30 AM
qcolombet requested changes to this revision.Thu, May 30, 9:31 AM
This revision now requires changes to proceed.Thu, May 30, 9:31 AM
skan abandoned this revision.Thu, May 30, 5:55 PM

RegClassOrRegBank is of one of the two classes, but it can still be nullptr, e.g., before regbankselect.
This patch has been reverted in llvm trunk.