[PATCH] D73985: [bpf] zero extension is required in BPF implementaiton so remove <<=32 >>=32

Yonghong Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 25 23:25:43 PDT 2020


yonghong-song added a comment.

> Seems something went wrong here? I'll look into it tomorrow but its not what I expected on two cases. First I expected a jmp32 op and second that last optimization seems wrong?

I checked the code. I agree with you that optimization seems not right.
In `BPFMIPeephole.cpp`,  we have

  // Eliminate identical move:
  //
  //   MOV rA, rA
  //
  // This is particularly possible to happen when sub-register support
  // enabled. The special type cast insn MOV_32_64 involves different
  // register class on src (i32) and dst (i64), RA could generate useless
  // instruction due to this.
  unsigned Opcode = MI.getOpcode();
  if (Opcode == BPF::MOV_32_64 ||
      Opcode == BPF::MOV_rr || Opcode == BPF::MOV_rr_32) {
    Register dst = MI.getOperand(0).getReg();
    Register src = MI.getOperand(1).getReg();
  
    if (Opcode == BPF::MOV_32_64)
      dst = TRI->getSubReg(dst, BPF::sub_32);
  
    if (dst != src)
      continue;
  
    ToErase = &MI;
    RedundantMovElemNum++;
    Eliminated = true;
  }

`MOV_32_64` actually problematic here as it is not a simple register copy, it has side effect.
The same as `MOV_rr_32`. it also has a side effect to zero out the top bits. We cannot simply remove
them.

What we need to do is to further trace def/use chain, unless the 32bit reg already has upper bits
zeroed, we should not do the optimization.

Will work on a fix ASAP. Thanks for spotting the problem!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73985





More information about the llvm-commits mailing list