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

[John Fastabend via Phabricator via llvm-commits]

jrfastab added a comment.

@yonghong-song I can update the tests but I'm not completely following the CHECKs in 32-bit-subreg-peephole.ll

  ; long long select_u(unsigned a, unsigned b, long long c, long long d)
  ; {
  ;   if (a > b)
  ;     return c;
  ;   else
  ;     return d;
  ; }

with corresponding checks,

  ; Function Attrs: norecurse nounwind readnone
  define dso_local i64 @select_u(i32 %a, i32 %b, i64 %c, i64 %d) local_unnamed_addr #0 {
  ; CHECK-LABEL: select_u:
  entry:
    %cmp = icmp ugt i32 %a, %b
    %c.d = select i1 %cmp, i64 %c, i64 %d
  ; CHECK: r{{[0-9]+}} <<= 32
  ; CHECK-NEXT: r{{[0-9]+}} >>= 32
  ; CHECK: if r{{[0-9]+}} {{<|>}} r{{[0-9]+}} goto
    ret i64 %c.d
  } 

Couple questions if you happen to know could save some time on my side. First why does this not use a 32-bit compare, %a is 32-bits and %b is 32-bit seems a JMP32 instruction should work OK. Here is the assembly generated,

  # %bb.0:                                # %entry
          r0 = r3
          r1 <<= 32
          r1 >>= 32
          r2 <<= 32
          r2 >>= 32
          if r1 > r2 goto LBB0_2
  # %bb.1:                                # %entry
          r0 = r4
  LBB0_2:                                 # %entry
          exit
  .Lfunc_end0:

Next with the patch I created here we drop the zext but it still does not use the alu32 ops. Here is the asm,

  # %bb.0:                                # %entry
          r0 = r3
          if r1 > r2 goto LBB0_2
  # %bb.1:                                # %entry
          r0 = r4
  LBB0_2:                                 # %entry
          exit

This is probably not correct because how do we "know" the upper bits are zero seems we still need to zero them. Looking at the passes before peephole optimization,

  # *** IR Dump After BPF PreEmit Checking ***:
  # Machine code for function select_u: NoPHIs, TracksLiveness, NoVRegs
  Function Live Ins: $w1, $w2, $r3, $r4
  
  bb.0.entry:
    successors: %bb.1(0x40000000), %bb.2(0x40000000); %bb.1(50.00%), %bb.2(50.00%)
    liveins: $r3, $r4, $w1, $w2
    $r0 = MOV_rr $r3
    $r1 = MOV_32_64 killed $w1
    $r2 = MOV_32_64 killed $w2
    JUGT_rr killed $r1, killed $r2, %bb.2
  
  bb.1.entry:
  ; predecessors: %bb.0
    successors: %bb.2(0x80000000); %bb.2(100.00%)
    liveins: $r4
    $r0 = MOV_rr killed $r4
  
  bb.2.entry:
  ; predecessors: %bb.0, %bb.1
    liveins: $r0
    RET implicit $r0
  
  # End machine code for function select_u.

And then after optimization,

  # *** IR Dump After BPF PreEmit Peephole Optimization ***:
  # Machine code for function select_u: NoPHIs, TracksLiveness, NoVRegs
  Function Live Ins: $w1, $w2, $r3, $r4
  
  bb.0.entry:
    successors: %bb.1(0x40000000), %bb.2(0x40000000); %bb.1(50.00%), %bb.2(50.00%)
    liveins: $r3, $r4, $w1, $w2
    $r0 = MOV_rr $r3
    JUGT_rr killed $r1, killed $r2, %bb.2
  
  bb.1.entry:
  ; predecessors: %bb.0
    successors: %bb.2(0x80000000); %bb.2(100.00%)
    liveins: $r4
    $r0 = MOV_rr killed $r4
  
  bb.2.entry:
  ; predecessors: %bb.0, %bb.1
    liveins: $r0
    RET implicit $r0
  
  # End machine code for function select_u.

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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73985