[PATCH] D157805: [BPF] Reset machine register kill mark in BPFMISimplifyPatchable

[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 on the generated object files (I checked for cpuv4).


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

  struct t {
    unsigned long a;
  } __attribute__((preserve_access_index));
  
  void foo(volatile struct t *t, volatile unsigned long *p) {
    *p = t->a;
    *p = t->a;
  }

Causes an assertion:

  $ clang -g -O2 -c --target=bpf -mcpu=v2 t2.c -o /dev/null
  
  # After BPF PreEmit SimplifyPatchable
  # Machine code for function foo: IsSSA, TracksLiveness
  Function Live Ins: $r1 in %0, $r2 in %1
  
  bb.0.entry:
    liveins: $r1, $r2
    DBG_VALUE $r1, $noreg, !"t", !DIExpression()
    DBG_VALUE $r2, $noreg, !"p", !DIExpression()
    %1:gpr = COPY $r2
    DBG_VALUE %1:gpr, $noreg, !"p", !DIExpression()
    %0:gpr = COPY $r1
    DBG_VALUE %0:gpr, $noreg, !"t", !DIExpression()
    %2:gpr = LD_imm64 @"llvm.t:0:0$0:0"
    %4:gpr = ADD_rr %0:gpr(tied-def 0), killed %2:gpr
    %5:gpr = CORE_LD 344, %0:gpr, @"llvm.t:0:0$0:0"
    STD killed %5:gpr, %1:gpr, 0
    %7:gpr = ADD_rr %0:gpr(tied-def 0), killed %2:gpr
    %8:gpr = CORE_LD 344, %0:gpr, @"llvm.t:0:0$0:0"
    STD killed %8:gpr, %1:gpr, 0
    RET
  
  # End machine code for function foo.
  
  *** Bad machine code: Using a killed virtual register ***
  - function:    foo
  - basic block: %bb.0 entry (0x6210000e6690)
  - instruction: %7:gpr = ADD_rr %0:gpr(tied-def 0), killed %2:gpr
  - operand 2:   killed %2:gpr

This happens because of the way
BPFMISimplifyPatchable::processDstReg() updates second operand of the
`ADD_rr` instruction. Code before `BPFMISimplifyPatchable`:

  .-> %2:gpr = LD_imm64 @"llvm.t:0:0$0:0"
  |
  |`----------------.
  |   %3:gpr = LDD %2:gpr, 0
  |   %4:gpr = ADD_rr %0:gpr(tied-def 0), killed %3:gpr <--- (1)
  |   %5:gpr = LDD killed %4:gpr, 0       ^^^^^^^^^^^^^
  |   STD killed %5:gpr, %1:gpr, 0        this is updated
   `----------------.
      %6:gpr = LDD %2:gpr, 0
      %7:gpr = ADD_rr %0:gpr(tied-def 0), killed %6:gpr <--- (2)
      %8:gpr = LDD killed %7:gpr, 0       ^^^^^^^^^^^^^
      STD killed %8:gpr, %1:gpr, 0        this is updated

Instructions (1) and (2) would be updated to:

  ADD_rr %0:gpr(tied-def 0), killed %2:gpr

The `killed` mark is inherited from machine operands `killed %3:gpr`
and `killed %6:gpr` which are updated inplace by `processDstReg()`.

This commit updates `processDstReg()` reset kill marks for updated
machine operands to keep liveness information conservatively correct.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157805

Files:
  llvm/lib/Target/BPF/BPFMISimplifyPatchable.cpp
  llvm/test/CodeGen/BPF/CORE/simplify-patchable-liveness-bug.ll

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