Page MenuHomePhabricator

[BPF] add support for 32 bit registers in inline asm
ClosedPublic

Authored by alessandrod on May 8 2021, 5:52 PM.

Details

Summary

Add "w" constraint type which allows selecting 32 bit registers.

32 bit registers were added in https://reviews.llvm.org/rGca31c3bb3ff149850b664838fbbc7d40ce571879.

Diff Detail

Event Timeline

alessandrod created this revision.May 8 2021, 5:52 PM
alessandrod requested review of this revision.May 8 2021, 5:52 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMay 8 2021, 5:52 PM

Generally looks good. One suggestion and a few nits. Thanks!

clang/lib/Basic/Targets/BPF.cpp
54

-mcpu=v3 also enables alu32. See below llvm code

void BPFSubtarget::initSubtargetFeatures(StringRef CPU, StringRef FS) {
  if (CPU == "probe")
    CPU = sys::detail::getHostCPUNameForBPF();
  if (CPU == "generic" || CPU == "v1")
    return;
  if (CPU == "v2") {
    HasJmpExt = true;
    return;
  }
  if (CPU == "v3") {
    HasJmpExt = true;
    HasJmp32 = true;
    HasAlu32 = true;
    return;
  }
}

Could you add a check with setCPU() to set HasAlu32 = true if it is v3? This way, when people directly use -mcpu=v3 in clang command line, 'w' constraint is also supported.

60

empty line here.

clang/test/CodeGen/bpf-inline-asm.c
6

I assume test_generic_constraints() will pass even without this patch, but it is a good addition for clang side of bpf inline-asm support, so I think it is okay to be included in this patch.

33

empty line at the end of file.

llvm/test/CodeGen/BPF/inlineasm-wreg.ll
1

Let us remove the empty line in the top of the test file.

20

empty line at the end of file.

Enable "w" constraint when -mcpu=v3 and fix whitespace.

yonghong-song accepted this revision.May 15 2021, 9:25 AM

LGTM except one minor issue. Please do address the nit before merging. Thanks!

clang/test/CodeGen/bpf-inline-asm.c
6

The var is not used and can be removed.

This revision is now accepted and ready to land.May 15 2021, 9:25 AM

Remove unused global from test

LGTM except one minor issue. Please do address the nit before merging. Thanks!

Done, thanks for the review! I don't have commit access so someone will have to merge this for me :)

Sure. I will merge the patch for you.

This revision was automatically updated to reflect the committed changes.