[PATCH] D129402: [RISCV] Teach shouldConvertConstantLoadToIntImm that constant materialization can use constant pools.

Craig Topper via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 11 09:41:27 PDT 2022


craig.topper added a comment.

In D129402#3642411 <https://reviews.llvm.org/D129402#3642411>, @asb wrote:

> Hi Craig - this seems to be causing some code quality regressions. Here is a relatively small (but not reduced) test case:
>
>   --- a/output_rv64imafdc_lp64d_Os/20120207-1.s
>   +++ b/output_rv64imafdc_lp64d_Os/20120207-1.s
>   @@ -2,24 +2,41 @@
>           .attribute      4, 16
>           .attribute      5, "rv64i2p0_m2p0_a2p0_f2p0_d2p0_c2p0"
>           .file   "20120207-1.c"
>   -       .section        .sdata,"aw", at progbits
>   -       .p2align        3                               # -- Begin function test
>   -.LCPI0_0:
>   -       .quad   3978425819141910832             # 0x3736353433323130
>   -       .text
>   -       .globl  test
>   +       .globl  test                            # -- Begin function test
>           .p2align        1
>           .type   test, at function
>    test:                                   # @test
>    # %bb.0:                                # %entry
>           addi    sp, sp, -16
>           lui     a1, 4
>   -       lui     a2, %hi(.LCPI0_0)
>   -       ld      a2, %lo(.LCPI0_0)(a2)
>           addiw   a1, a1, -1736
>           sh      a1, 8(sp)
>   +       lui     a1, %hi(.L.str)
>   +       addi    a2, a1, %lo(.L.str)
>   +       lbu     a3, 1(a2)
>   +       lbu     a1, %lo(.L.str)(a1)
>   +       lbu     a4, 3(a2)
>   +       lbu     a5, 2(a2)
>   +       slli    a3, a3, 8
>   +       or      a1, a1, a3
>   +       slli    a3, a4, 8
>   +       or      a3, a3, a5
>   +       lbu     a4, 5(a2)
>   +       slli    a3, a3, 16
>   +       or      a1, a1, a3
>   +       lbu     a3, 4(a2)
>   +       slli    a4, a4, 8
>   +       lbu     a5, 7(a2)
>   +       lbu     a2, 6(a2)
>   +       or      a3, a3, a4
>           sb      zero, 10(sp)
>   -       sd      a2, 0(sp)
>   +       slli    a4, a5, 8
>   +       or      a2, a2, a4
>   +       slli    a2, a2, 16
>   +       or      a2, a2, a3
>   +       slli    a2, a2, 32
>   +       or      a1, a1, a2
>   +       sd      a1, 0(sp)
>           mv      a1, sp
>           add     a0, a0, a1
>           lbu     a0, -1(a0)
>
>
>
>   ; ModuleID = './output_rv64imafdc_lp64d_Os/20120207-1.bc'
>   source_filename = "20120207-1.c"
>   target datalayout = "e-m:e-p:64:64-i64:64-i128:128-n64-S128"
>   target triple = "riscv64-unknown-linux-gnu"
>   
>   @.str = private unnamed_addr constant [11 x i8] c"0123456789\00", align 1
>   
>   ; Function Attrs: noinline nounwind optsize
>   define dso_local zeroext i8 @test(i32 noundef signext %a) #0 {
>   entry:
>     %a.addr = alloca i32, align 4
>     %buf = alloca [16 x i8], align 1
>     %output = alloca ptr, align 8
>     store i32 %a, ptr %a.addr, align 4, !tbaa !3
>     call void @llvm.lifetime.start.p0(i64 16, ptr %buf) #5
>     call void @llvm.lifetime.start.p0(i64 8, ptr %output) #5
>     %arraydecay = getelementptr inbounds [16 x i8], ptr %buf, i64 0, i64 0
>     store ptr %arraydecay, ptr %output, align 8, !tbaa !7
>     %arrayidx = getelementptr inbounds [16 x i8], ptr %buf, i64 0, i64 0
>     %call = call ptr @strcpy(ptr noundef %arrayidx, ptr noundef @.str) #6
>     %0 = load i32, ptr %a.addr, align 4, !tbaa !3
>     %1 = load ptr, ptr %output, align 8, !tbaa !7
>     %idx.ext = sext i32 %0 to i64
>     %add.ptr = getelementptr inbounds i8, ptr %1, i64 %idx.ext
>     store ptr %add.ptr, ptr %output, align 8, !tbaa !7
>     %2 = load ptr, ptr %output, align 8, !tbaa !7
>     %add.ptr1 = getelementptr inbounds i8, ptr %2, i64 -1
>     store ptr %add.ptr1, ptr %output, align 8, !tbaa !7
>     %3 = load ptr, ptr %output, align 8, !tbaa !7
>     %arrayidx2 = getelementptr inbounds i8, ptr %3, i64 0
>     %4 = load i8, ptr %arrayidx2, align 1, !tbaa !9
>     call void @llvm.lifetime.end.p0(i64 8, ptr %output) #5
>     call void @llvm.lifetime.end.p0(i64 16, ptr %buf) #5
>     ret i8 %4
>   }
>   
>   ; Function Attrs: argmemonly nocallback nofree nosync nounwind willreturn
>   declare void @llvm.lifetime.start.p0(i64 immarg, ptr nocapture) #1
>   
>   ; Function Attrs: optsize
>   declare dso_local ptr @strcpy(ptr noundef, ptr noundef) #2
>   
>   ; Function Attrs: argmemonly nocallback nofree nosync nounwind willreturn
>   declare void @llvm.lifetime.end.p0(i64 immarg, ptr nocapture) #1
>   
>   ; Function Attrs: nounwind optsize
>   define dso_local signext i32 @main() #3 {
>   entry:
>     %retval = alloca i32, align 4
>     store i32 0, ptr %retval, align 4
>     %call = call zeroext i8 @test(i32 noundef signext 2) #6
>     %conv = zext i8 %call to i32
>     %cmp = icmp ne i32 %conv, 49
>     br i1 %cmp, label %if.then, label %if.end
>   
>   if.then:                                          ; preds = %entry
>     call void @abort() #7
>     unreachable
>   
>   if.end:                                           ; preds = %entry
>     ret i32 0
>   }
>   
>   ; Function Attrs: noreturn optsize
>   declare dso_local void @abort() #4
>   
>   attributes #0 = { noinline nounwind optsize "frame-pointer"="none" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+64bit,+a,+c,+d,+f,+m,+relax,-save-restore" }
>   attributes #1 = { argmemonly nocallback nofree nosync nounwind willreturn }
>   attributes #2 = { optsize "frame-pointer"="none" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+64bit,+a,+c,+d,+f,+m,+relax,-save-restore" }
>   attributes #3 = { nounwind optsize "frame-pointer"="none" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+64bit,+a,+c,+d,+f,+m,+relax,-save-restore" }
>   attributes #4 = { noreturn optsize "frame-pointer"="none" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+64bit,+a,+c,+d,+f,+m,+relax,-save-restore" }
>   attributes #5 = { nounwind }
>   attributes #6 = { optsize }
>   attributes #7 = { noreturn optsize }
>   
>   !llvm.module.flags = !{!0, !1, !2}
>   
>   !0 = !{i32 1, !"wchar_size", i32 4}
>   !1 = !{i32 1, !"target-abi", !"lp64d"}
>   !2 = !{i32 1, !"SmallDataLimit", i32 8}
>   !3 = !{!4, !4, i64 0}
>   !4 = !{!"int", !5, i64 0}
>   !5 = !{!"omnipotent char", !6, i64 0}
>   !6 = !{!"Simple C/C++ TBAA"}
>   !7 = !{!8, !8, i64 0}
>   !8 = !{!"any pointer", !5, i64 0}
>   !9 = !{!5, !5, i64 0}

Thanks Alex. I pushed 1a2bd44b77c2dd0c2aabf5e27e30eb17c4715832 <https://reviews.llvm.org/rG1a2bd44b77c2dd0c2aabf5e27e30eb17c4715832> to restore the old behavior when enableScalarMem is false to restore the old behavior while I investigate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129402



More information about the llvm-commits mailing list