[llvm-commits] [llvm] r109359 - in /llvm/trunk: include/llvm/Target/TargetLowering.h lib/Target/ARM/ARM.h lib/Target/ARM/ARMGlobalMerge.cpp lib/Target/ARM/ARMISelLowering.cpp lib/Target/ARM/ARMISelLowering.h lib/Target/ARM/ARMTargetMachine.cpp li

[Anton Korobeynikov]

Hi, Chris

> Sorry for the delay reviewing this.  My general thought on this is that it should be a target-independent pass.  PowerPC would certainly benefit from this as well, probably x86 in PIC mode as well, among others.  Can this be generalized to support other targets?
Yes, surely.

> Also, you added no regression tests for this.  Please fix this, this is a major new feature, and we want to make sure it is not broken in the future.
Ok, will add.

> Could you just use the existing hooks that LSR uses to determine reg+imm offsets?  The "offset from a global" depends on the type being accessed and many other things, so it isn't clear how to define this hook.  Using the existing LSR hooks seems beneficial because if you end up turning [reg1+reg2] into tmp = reg1+imm [tmp+reg2] addressing, you actually don't save a register.  I understand that you're not modeling this yet, but in any case, fewer hooks is better.
Do you mean TLI::isLegalAddressingMode() ? If yes, then it's too
inflexible, it allows only simple value types and will return nothing
on e.g. array of structs.

-- 
With best regards, Anton Korobeynikov
Faculty of Mathematics and Mechanics, Saint Petersburg State University