Page MenuHomePhabricator

[RISCV] Scale scalably-typed split argument offsets by VSCALE
ClosedPublic

Authored by frasercrmck on May 27 2021, 9:07 AM.

Details

Summary

This patch fixes a bug in lowering scalable-vector types in RISC-V's
main calling convention. When scalable-vector types are split and passed
indirectly, the target is responsible for scaling the offset --
initially set to the known-minimum store size -- by the scalable factor.

Before this we were issuing overlapping loads or stores to the different
parts, leading to incorrect codegen.

Credit to @HsiangKai for spotting this.

Diff Detail

Event Timeline

frasercrmck created this revision.May 27 2021, 9:07 AM
frasercrmck requested review of this revision.May 27 2021, 9:07 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 27 2021, 9:07 AM
  • remove FIXMEs, on account of it being.. fixed
frasercrmck added inline comments.May 27 2021, 9:20 AM
llvm/test/CodeGen/RISCV/rvv/calling-conv.ll
50

I'd better dig into these 32s (presumably the "loc mem offset"?) and make sure they're going to "scale" to multiple split arguments.

frasercrmck added inline comments.May 27 2021, 9:39 AM
llvm/test/CodeGen/RISCV/rvv/calling-conv.ll
50

Looks like they're fine as they're regular stack FIs which are correctly typed as scalable-vector.

define <vscale x 32 x i32> @caller(<vscale x 32 x i32> %x) {
; RV64-NEXT:    ret
  %a = call <vscale x 32 x i32> @callee(<vscale x 32 x i32> zeroinitializer, <vscale x 32 x i32> %x, <vscale x 32 x i32> zeroinitializer)
stack:
  - { id: 0, name: '', type: default, offset: 0, size: 128, alignment: 128,
      stack-id: scalable-vector, callee-saved-register: '', callee-saved-restored: true,
      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
  - { id: 1, name: '', type: default, offset: 0, size: 128, alignment: 128,
      stack-id: scalable-vector, callee-saved-register: '', callee-saved-restored: true,
      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }

...
  bb.0 (%ir-block.0):
    liveins: $v8m8, $v16m8

    %1:vrm8 = COPY $v16m8
    %0:vrm8 = COPY $v8m8
    %2:gpr = PseudoReadVLENB
    %3:gpr = SLLI killed %2, 3
    %4:gpr = ADDI %stack.1, 0
    %5:gpr = ADD %4, %3
    ADJCALLSTACKDOWN 0, 0, implicit-def dead $x2, implicit $x2
    %6:vrm8 = PseudoVMV_V_I_M8 0, $x0, 5
    VS8R_V %6, killed %5 :: (store unknown-size into %stack.1, align 64)
    %7:gpr = ADDI %stack.0, 0
    %8:gpr = ADD %7, %3
    VS8R_V %1, killed %8 :: (store unknown-size into %stack.0, align 64)
    VS8R_V %6, %stack.1 :: (store unknown-size into %stack.1, align 128)
    VS8R_V %0, %stack.0 :: (store unknown-size into %stack.0, align 128)
    $v8m8 = COPY %6
    $v16m8 = COPY %6
    $x10 = COPY %7
    $x12 = COPY %4
    PseudoCALL target-flags(riscv-plt) @callee, csr_ilp32_lp64, implicit-def dead $x1, implicit $v8m8, implicit $v16m8, implicit $x10, implicit $x12, implicit-def $x2, implicit-def $v8m8, implicit-def $v16m8
This revision is now accepted and ready to land.May 30 2021, 7:27 AM
This revision was landed with ongoing or failed builds.May 31 2021, 2:51 AM
This revision was automatically updated to reflect the committed changes.