[PATCH] D157806: [BPF] Fix in/out argument constraints for CORE_MEM instructions

[Eduard Zingerman via Phabricator via llvm-commits]

eddyz87 created this revision.
Herald added a subscriber: hiraditya.
Herald added a project: All.
eddyz87 published this revision for review.
eddyz87 added a reviewer: yonghong-song.
eddyz87 added a comment.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Hi Yonghong,

Could you please take a look at this revision?
All BPF Kernel selftests are passing and there are no changes in the generated object files (I checked for cpuv4).
This is the reason for testbot failures reported <https://reviews.llvm.org/D140804#4578547> for D140804 <https://reviews.llvm.org/D140804>.
(The test case in D140804 <https://reviews.llvm.org/D140804> was written in a way that triggers this failure).


When LLVM is build with `LLVM_ENABLE_EXPENSIVE_CHECKS=ON` option the
following C code snippet:

  struct t {
    int a;
  } __attribute__((preserve_access_index));
  
  void test(struct t *t) {
    t->a = 42;
  }

Causes an assertion:

  llvm
  $ clang -g -O2 -c --target=bpf -mcpu=v2 t.c -o /dev/null
  
  Function Live Ins: $r1 in %0
  
  bb.0.entry:
    liveins: $r1
    DBG_VALUE $r1, $noreg, !"t", ...
    %0:gpr = COPY $r1
    DBG_VALUE %0:gpr, $noreg, !"t", ...
    %1:gpr = LD_imm64 @"llvm.t:0:0$0:0"
    %3:gpr = ADD_rr %0:gpr(tied-def 0), killed %1:gpr
    %4:gpr = MOV_ri 42
    CORE_MEM killed %4:gpr, 411, %0:gpr, @"llvm.t:0:0$0:0", ...
    RET debug-location !25; t.c:7:1
  
  *** Bad machine code: Explicit definition marked as use ***
  - function:    test
  - basic block: %bb.0 entry (0x6210000d8a90)
  - instruction: CORE_MEM killed %4:gpr, 411, %0:gpr, @"llvm.t:0:0$0:0", ...
  - operand 0:   killed %4:gpr

This happens because `CORE_MEM` instruction is defined to have output
operands:

  tablegen
    def CORE_MEM : TYPE_LD_ST<BPF_MEM.Value, BPF_W.Value,
                              (outs GPR:$dst),
                              (ins u64imm:$opcode, GPR:$src, u64imm:$offset),
                              "$dst = core_mem($opcode, $src, $offset)",
                              []>;

As documented in [1]:

> By convention, the LLVM code generator orders instruction operands
> so that all register definitions come before the register uses, even
> on architectures that are normally printed in other orders.

In other words, the first argument for `CORE_MEM` is considered to be
a "def", while in reality it is "use":

  llvm
    %1:gpr = LD_imm64 @"llvm.t:0:0$0:0"
    %3:gpr = ADD_rr %0:gpr(tied-def 0), killed %1:gpr
    %4:gpr = MOV_ri 42
     '---------------.
                     v
    CORE_MEM killed %4:gpr, 411, %0:gpr, @"llvm.t:0:0$0:0", ...

Here is how `CORE_MEM` is constructed in
`BPFMISimplifyPatchable::checkADDrr()`:

  cpp
      BuildMI(*DefInst->getParent(), *DefInst, DefInst->getDebugLoc(), TII->get(COREOp))
          .add(DefInst->getOperand(0)).addImm(Opcode).add(*BaseOp)
          .addGlobalAddress(GVal);

Note that first operand is constructed as `.add(DefInst->getOperand(0))`.

For `LD{D,W,H,B}` instructions the `DefInst->getOperand(0)` is a
destination register of a load, so instruction is constructed in
accordance with `outs` declaration.

For `ST{D,W,H,B}` instructions the `DefInst->getOperand(0)` is a
source register of a store (value to be stored), so instruction
violates the `outs` declaration.

This commit fixes the issue by splitting `CORE_MEM` in two
instructions: `CORE_ST`, `CORE_LD` with correct `outs`
specifications. (+ same changes for `CORE_ALU32_MEM`).

[1] https://llvm.org/docs/CodeGenerator.html#the-machineinstr-class


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157806

Files:
  llvm/lib/Target/BPF/BPFInstrInfo.td
  llvm/lib/Target/BPF/BPFMISimplifyPatchable.cpp
  llvm/lib/Target/BPF/BTFDebug.cpp
  llvm/test/CodeGen/BPF/CORE/offset-reloc-simplify-patchable-1.ll
  llvm/test/CodeGen/BPF/CORE/offset-reloc-simplify-patchable-2.ll
  llvm/test/CodeGen/BPF/CORE/offset-reloc-simplify-patchable-3.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D157806.549672.patch
Type: text/x-patch
Size: 30520 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230813/e88ea749/attachment.bin>