[llvm-commits] [llvm] r98409 - in /llvm/trunk/lib/Target/ARM: ARMConstantIslandPass.cpp ARMInstrFormats.td ARMInstrInfo.td ARMInstrThumb.td ARMInstrThumb2.td ARMInstrVFP.td ARMLoadStoreOptimizer.cpp AsmPrinter/ARMAsmPrinter.cpp Thumb1InstrInfo.cpp Thumb1RegisterInfo.cpp Thumb2SizeReduction.cpp

[Jakob Stoklund Olesen]

On Mar 12, 2010, at 5:08 PM, Bob Wilson wrote:

> Author: bwilson
> Date: Fri Mar 12 19:08:20 2010
> New Revision: 98409
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=98409&view=rev
> Log:
> Change ARM ld/st multiple instructions to have variant instructions for
> writebacks to the address register.  This gets rid of the hack that the
> first register on the list was the magic writeback register operand.  There
> was an implicit constraint that if that operand was not reg0 it had to match
> the base register operand.  The post-RA scheduler's antidependency breaker
> did not understand that constraint and sometimes changed one without the
> other.  This also fixes Radar 7495976 and should help the verifier work
> better for ARM code.
> 
> There are now new ld/st instructions explicit writeback operands and explicit
> constraints that tie those registers together.

Woot! This was a major blocker for the verifier on ARM.

> Modified: llvm/trunk/lib/Target/ARM/ARMLoadStoreOptimizer.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMLoadStoreOptimizer.cpp?rev=98409&r1=98408&r2=98409&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/ARM/ARMLoadStoreOptimizer.cpp (original)
> +++ llvm/trunk/lib/Target/ARM/ARMLoadStoreOptimizer.cpp Fri Mar 12 19:08:20 2010
...
> +  if (!DoMerge)
> +    return false;
> +
> +  unsigned NewOpc = getUpdatingLSMultipleOpcode(Opcode);
> +  MachineInstrBuilder MIB = BuildMI(MBB, MBBI, dl, TII->get(NewOpc))
> +    .addReg(Base, getDefRegState(true)) // WB base register
> +    .addReg(Base, getKillRegState(BaseKill));

For a two-addr instruction, the kill flag on the tied use operand is implied, and it is even invalid to have an explicit kill flag there.

The same issue appears in a couple of spots below.

I would like to get rid of that rule, but for now we are stuck with it.

Maybe BaseKill should become <def,dead> on the writeback operand, but in that case there was no need to convert to an *_UPD variant in the first place?

/jakob