Page MenuHomePhabricator

[PowerPC] Handle inline assembly clobber of link regsiter
ClosedPublic

Authored by stefanp on Apr 30 2021, 12:16 PM.

Details

Summary

This patch adds the handling of clobbers of the link register LR for inline
assembly.

This patch is to fix:
https://bugs.llvm.org/show_bug.cgi?id=50147

Diff Detail

Event Timeline

stefanp created this revision.Apr 30 2021, 12:16 PM
stefanp requested review of this revision.Apr 30 2021, 12:16 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 30 2021, 12:16 PM
stefanp added reviewers: lei, nemanjai, Restricted Project.Apr 30 2021, 12:18 PM
stefanp edited the summary of this revision. (Show Details)Apr 30 2021, 1:53 PM

I added this commit on top of a45fd436aef4d5712da99f8292f5d0b16794892c, built a master branch Linux kernel from ppc44x_defconfig, and booted in under QEMU 5.0.1 without any issues as before. Thanks for the quick fix!

You probably also want the following testcase:

define void @UseLR() {
  tail call void asm sideeffect "", "{lr}"(i32 1)
  ret void
}
stefanp updated this revision to Diff 343156.May 5 2021, 1:14 PM

Cleanup the source code according to clang format.

Minor changes for formatting.

You probably also want the following testcase:

define void @UseLR() {
  tail call void asm sideeffect "", "{lr}"(i32 1)
  ret void
}

I looked into doing this but PowerPC does not have enough support for special registers (like lr) for us to be able to do this test correctly. The backend does not recognize the copy of the value 1 to the LR register and it does not know how to correctly spill special registers. This is on our TODO list and we will address these issues in a separate patch. Also, at this point, the clang frontend does not recognize the lr register as a valid input register anyway. (This work is also on our TODO list.) The plan is to address both of these deficiencies in future patches.

Basically, if we try this example from C source we get:

$ cat useLR.c

void foo() {
  asm(""
      : 
      : "l"(1));
}

$ clang -O2 -S useLR.c
clang: /data/stefanp/Wyvern/main/llvm-project/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:8657: void llvm::SelectionDAGBuilder::visitInlineAsm(const llvm::CallBase &): Assertion `(OpInfo.ConstraintType == TargetLowering::C_RegisterClass || OpInfo.ConstraintType == TargetLowering::C_Register) && "Unknown constraint type!"' failed.
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace, preprocessed source, and associated run script.

We will add this test when we add more support for special regsiters on PowerPC.

Is there anything we can do to help get this merged? It would be nice to have this fixed in LLVM 12.0.1, which is going to have its first release candidate soon.

nemanjai accepted this revision.May 12 2021, 12:04 PM

LGTM other than the allocatable nit. Let's go with this fix for now to unblock the kernel build and we'll follow-up with a more complete fix for other reserved registers later.

llvm/lib/Target/PowerPC/PPCRegisterInfo.td
413

Should these have the let isAllocatable = 0 as well? Seems like they should.

This revision is now accepted and ready to land.May 12 2021, 12:04 PM
stefanp updated this revision to Diff 345100.May 13 2021, 5:42 AM

Added missing let isAllocatable = 0.

This revision was landed with ongoing or failed builds.May 13 2021, 5:43 AM
This revision was automatically updated to reflect the committed changes.