CTSRD-CHERI / llvm-project

Fork of LLVM adding CHERI support
48 stars 41 forks source link

[ABI BREAK] Should pass pointers-to-members directly for purecap RISC-V #573

Open arichardson opened 3 years ago

arichardson commented 3 years ago

This probably won't have a huge impact on performance, so we should probably wait until we are breaking the ABI anyway and include this change.

Right now we pass pointers-to-members as an indirect { i8 addrspace(200)*, i64 } addrspace(200)* argument so we have to add a stack allocation and setbounds instruction.

See https://cheri-compiler-explorer.cl.cam.ac.uk/z/3eETGd:

struct A {
  virtual long vf1() { return 1; }
  virtual long vf2() { return 2; }
};

long use_p2m(A* a, long (A::*fp)());
int pass_p2m() {
  return use_p2m(nullptr, &A::vf2);
}

->

pass_p2m():                           # @pass_p2m()
        cincoffset      csp, csp, -48
        csc     cra, 32(csp)                    # 16-byte Folded Spill
        cincoffset      ca0, cnull, 17
        csc     ca0, 0(csp)
        csd     zero, 16(csp)
        cincoffset      ca0, csp, 0
        csetbounds      ca1, ca0, 32
.LBB0_1:                                # %entry
        auipcc  ca2, %captab_pcrel_hi(_Z7use_p2mP1AMS_FlvE)
        clc     ca2, %pcrel_lo(.LBB0_1)(ca2)
        cmove   ca0, cnull
        cjalr   ca2
        sext.w  a0, a0
        clc     cra, 32(csp)                    # 16-byte Folded Reload
        cincoffset      csp, csp, 48
        cret

whereas plain RISC-V and MIPS pass it directly:

pass_p2m():                           # @pass_p2m()
        addi    sp, sp, -16
        sd      ra, 8(sp)                       # 8-byte Folded Spill
        addi    a1, zero, 9
        mv      a0, zero
        mv      a2, zero
        call    use_p2m(A*, long (A::*)())
        sext.w  a0, a0
        ld      ra, 8(sp)                       # 8-byte Folded Reload
        addi    sp, sp, 16
        ret

MIPS:

pass_p2m():                           # @pass_p2m()
        cincoffset      $c11, $c11, -16
        csc     $c17, $zero, 0($c11)            # 16-byte Folded Spill
        lui     $1, %pcrel_hi(_CHERI_CAPABILITY_TABLE_-8)
        daddiu  $1, $1, %pcrel_lo(_CHERI_CAPABILITY_TABLE_-4)
        cgetpccincoffset        $c1, $1
        clcbi   $c12, %capcall20(_Z7use_p2mP1AMS_FlvE)($c1)
        cincoffset      $c4, $cnull, 16
        daddiu  $4, $zero, 1
        cjalr   $c12, $c17
        cgetnull        $c3
        sll     $2, $2, 0
        clc     $c17, $zero, 0($c11)            # 16-byte Folded Reload
        cjr     $c17
        cincoffset      $c11, $c11, 16
jrtc27 commented 3 years ago

How common are pointers-to-members? If it's just a bit of Qt stuff then I personally would be fine with not doing a proper transition for that ABI break as nobody other than you will notice...

jrtc27 commented 3 years ago

Oh, is this just a specific case of struct { void *; long; } gets passed indirectly rather than treated like other register pair structs, as currently it only recognises {GPR,FPR}x{GPR,FPR} pairs?

arichardson commented 3 years ago

I feel like there might be some libc++ internals that are affected by this, but I haven't verified it.

And yes, we can probably fix this by classifying struct { void *; long; } as using two GPRs, but then it would be a bigger ABI break: https://cheri-compiler-explorer.cl.cam.ac.uk/z/hdEWje

arichardson commented 3 years ago

And we should probably also return pairs in registers nstead of sret: https://cheri-compiler-explorer.cl.cam.ac.uk/z/s96WTY

I think that might have a perf impact that is worth an ABI break.

jrtc27 commented 3 years ago

Yeah, returns are defined to be "how it would be passed as the first argument" so the two are the same.

And yes, it's what I always meant to do, it's just a real pain to get that code right when you have FPRs to deal with too so punted...

arichardson commented 2 years ago

Looks like RISC-V also passes FP values in x registers like AArch64? https://cheri-compiler-explorer.cl.cam.ac.uk/z/Prccqn