[PATCH] D130481: [RISCV] Add the GlobalMerge pass (disabled by default)

[Alex Bradbury via Phabricator via llvm-commits]

asb added inline comments.


================
Comment at: llvm/test/CodeGen/RISCV/global-merge-offset.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: sed 's/ArrSize/100/g' %s | llc -mtriple=riscv32 -riscv-enable-global-merge \
+; RUN:   -verify-machineinstrs | FileCheck %s
----------------
craig.topper wrote:
> asb wrote:
> > reames wrote:
> > > The use of sed here is simply overly complicating things.
> > > 
> > > Please add a second test to this file which uses a separate global with an alternate size.  
> > The use of sed is because it's not very easy to construct a test (in a way that remains easily readable at least) that demonstrates the global merging behaviour.
> > 
> > The file I started with did:
> > 
> > ```
> > @ga1 = dso_local global [510 x i32] zeroinitializer, align 4
> > @gi1 = dso_local global i32 0, align 4
> > 
> > ; TODO: It would be better for codesize if the final store below was
> > ; `sw a0, 0(a2)`.
> > 
> > define void @f_mergeable(i32 %a) nounwind {
> > ; CHECK-LABEL: f_mergeable:
> > ; CHECK:       # %bb.0:
> > ; CHECK-NEXT:    lui a1, %hi(ga1+2040)
> > ; CHECK-NEXT:    sw a0, %lo(ga1+2040)(a1)
> > ; CHECK-NEXT:    lui a1, %hi(.L_MergedGlobals)
> > ; CHECK-NEXT:    sw a0, %lo(.L_MergedGlobals)(a1)
> > ; CHECK-NEXT:    ret
> >   %ga1_end = getelementptr inbounds [510 x i32], ptr @ga1, i32 0, i64 510
> >   store i32 %a, ptr %ga1_end, align 4
> >   store i32 %a, ptr @gi1, align 4
> >   ret void
> > }
> > 
> > @ga2 = dso_local global [511 x i32] zeroinitializer, align 4
> > @gi2 = dso_local global i32 0, align 4
> > 
> > define void @f_unmergeable(i32 %a) nounwind {
> > ; CHECK-LABEL: f_unmergeable:
> > ; CHECK:       # %bb.0:
> > ; CHECK-NEXT:    lui a1, %hi(ga2+2040)
> > ; CHECK-NEXT:    sw a0, %lo(ga2+2040)(a1)
> > ; CHECK-NEXT:    lui a1, %hi(.L_MergedGlobals+4)
> > ; CHECK-NEXT:    sw a0, %lo(.L_MergedGlobals+4)(a1)
> > ; CHECK-NEXT:    ret
> >   %ga2_end = getelementptr inbounds [511 x i32], ptr @ga2, i32 0, i64 510
> >   store i32 %a, ptr %ga2_end, align 4
> >   store i32 %a, ptr @gi2, align 4
> >   ret void
> > }
> > ```
> > 
> > But you can see the globals merging pass merged gi1 and gi2, making the test not so useful (room for improved heuristics in the pass too perhaps...).
> So the options are sed or two test files? @reames which do you prefer?
Yes, I think those are the feasible options.


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

https://reviews.llvm.org/D130481