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

Sun Aug 13 14:57:06 PDT 2023

yonghong-song added a comment.

In D157805#4583326 <https://reviews.llvm.org/D157805#4583326>, @eddyz87 wrote:

> In D157805#4583287 <https://reviews.llvm.org/D157805#4583287>, @yonghong-song wrote:
>
>> LGTM. Thanks for detailed comments to show why generated code is incorrect. Do you think we should backport this fix to llvm17?
>
> On the one hand the code dates back to Dec 2019 and I don't see it affecting code generation in practice (at-least for our selftests). On the other hand it is really easy to backport. I'd say we should backport it.

Sounds good. Let us do it.

> In D157805#4583292 <https://reviews.llvm.org/D157805#4583292>, @yonghong-song wrote:
>
>> With LLVM_ENABLE_EXPENSIVE_CHECKS, even with this patch, there are still some selftest llvm crashes. I guess you are probably working on this (e.g. D157806 <https://reviews.llvm.org/D157806>).
>
> Yes, I'm aware of two more issues, the one addressed by D157806 <https://reviews.llvm.org/D157806> and the one in "BPF MachineSSA Peephole Optimization For TRUNC Eliminate":
>
>   *** Bad machine code: Illegal virtual register for instruction ***
>   - function:    _dissect
>   - basic block: %bb.9 if.end10 (0x621000e80d98)
>   - instruction: %24:gpr32 = MOV_rr %8:gpr32, debug-location !339; progs/bpf_flow.c:120:2 @[ progs/bpf_flow.c:161:9 ]
>   - operand 0:   %24:gpr32
>   Expected a GPR register, but got a GPR32 register
>
> Which I suppose is caused by `MOV_rr` instruction generated in the end of `BPFMIPeepholeTruncElim::eliminateTruncSeq()`. I'll submit a fix for this issue a bit later.

Sounds good. Thanks for discovering these issues and fixing them.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157805/new/

https://reviews.llvm.org/D157805