[PATCH] D129686: [RISCV] Reuse a materialised global address in preference to merging into a load/store

Mon Jan 30 12:11:51 PST 2023

reames added inline comments.
Herald added a subscriber: luke.

================
Comment at: llvm/test/CodeGen/RISCV/fold-addi-loadstore.ll:84
+; RV32C-NEXT:    lw a0, 0(a1)
+; RV32C-NEXT:    lw a1, 4(a1)
+; RV32C-NEXT:    ret
----------------
Looking at your test here, there's something interesting lurking.  You probably already knew this, but it took me a while staring at the code to figure this out.  

We're materializing two addresses here.  Those addresses are known to be four bytes apart, but we do not know that e.g. the high bits are common.  The two addresses could e.g. straddle a 12 bit aligned boundary, and thus the high bits of the second may not match those of the first.  If we did know this fact, we could use the common high bits and fold both low parts into loads.  (Your load_g_8 actually covers exactly that.)

So, conceptually, the Rv32 split case does look exactly like two merged i32 globals.  

Now, I do have to say that this test case may not matter.  The RV32 split i64 case should be 8 byte aligned, and thus not hit this.

I do see why this is interesting from a global merging perspective though.  Coming at this a bit differently, are we allowed to increase the alignment of the merged global?  If we are, we might make cases look like more load_g_8 than load_g_1.  

Another idea: Do we have any way to express the opposite of alignment?  That is, we don't care that an address is aligned *unless* the result would cross some boundary?  I remember we have such a concept for instructions (e.g. boundary align, thanks JCC errata!), but if could spell something like that for globals, then we could leave their addresses unaligned *unless* they crossed the boundary which allows the high bits to differ.  Haven't explored that much, so not sure what we have here.  

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

https://reviews.llvm.org/D129686