[PATCH] D114950: [RISCV] Promote large integers to constant pool

Alex Bradbury via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 9 06:16:44 PST 2021


asb added a comment.

Thanks for the patch!

This is generating illegal code in some cases, please try the following test case with RV64:

  $ cat tc.ll 
  define dso_local fp128 @foo(fp128 %x) {
  entry:
    %x.addr = alloca fp128, align 16
    store fp128 %x, fp128* %x.addr, align 16
    %0 = load fp128, fp128* %x.addr, align 16
    %1 = bitcast fp128 %0 to i128
    %2 = icmp slt i128 %1, 0
    %3 = zext i1 %2 to i64
    %cond = select i1 %2, fp128 0xL8469898CC51701B84000921FB54442D1, fp128 0xL00000000000000000000000000000000
    ret fp128 %cond
  }
  
  ; Function Attrs: nounwind
  define dso_local signext i32 @main() {
  entry:
    %retval = alloca i32, align 4
    store i32 0, i32* %retval, align 4
    %call = call fp128 @foo(fp128 0xL0000000000000000BFFF000000000000)
    %cmp = fcmp une fp128 %call, 0xL8469898CC51701B84000921FB54442D1
    br i1 %cmp, label %if.then, label %if.end
  
  if.then:                                          ; preds = %entry
    call void @abort()
    unreachable
  
  if.end:                                           ; preds = %entry
    ret i32 0
  }
  
  declare dso_local void @abort()

It produces a `ld	a3, zero(a3)` which is of course invalid.

There's a question of what to do about some of the imm.ll test cases. Perhaps it makes sense to add an option to either change the threshold or disable the constant pool, then some of these tests can still usefully check the materialisation logic and it's easier for people to experiment with different thresholds if their microarch might benefit? I'd be open to counter-arguments that it's not worth the hassle though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114950



More information about the llvm-commits mailing list