search

Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Missing memoperands prevents ARM load/store folding in a few cases #7972

Closed Quuxplusone closed 14 years ago

Quuxplusone commented 14 years ago
Bugzilla Link PR7529
Status RESOLVED FIXED
Importance P normal
Reported by Jakob Stoklund Olesen (stoklund@2pi.dk)
Reported on 2010-06-30 11:01:14 -0700
Last modified on 2010-07-12 11:44:25 -0700
Version trunk
Hardware All All
CC anton@korobeynikov.info, evan.cheng@apple.com, llvm-bugs@lists.llvm.org
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

Revision 107114 changed the ARMLoadStoreOptimizer pass to not touch loads and stores with missing memoperands. When there is no memoperand to say otherwise, the memory access could be unaligned or volatile.

This caused a small change in code generation - 0.2% of ldms and 0.1% of stms lost across the nightly test suite.

We should check why those loads and stores had no memoperands, and if we are missing some possible optimizations.

Quuxplusone commented 14 years ago

I've looked into this. The main issue is when the register allocator folded reloads from fixed slots the loads ended up with alignment 1. That's because by default all fixed slots have alignment of 1. This is overly conservative. DAG combiner will infer the correct alignments from stack pointer alignments and frame offset. But at register allocation time we are simply taking the frame object's alignments.

I think the right fix is to make sure the fixed objects have the inferred alignments when they are created. Does anyone see a problem with that?

Quuxplusone commented 14 years ago

Fixed: 107591. Jakob, please verify.

Quuxplusone commented 14 years ago 
I think your patch in 107591 is definitely correct, and it should provide more
ldm/stm folding, but the original 107114 only changed the behavior for loads
and stores without any memoperands.

107591 doesn't add memoperands to to loads and stores that don't have any,
right?
Quuxplusone commented 14 years ago

Right. I think that's the right behavior.

I looked a few changes due to 107114. They all look like they are related to remat of loads from fixed slots. Are there other missing cases?

Quuxplusone commented 14 years ago

I think that is al