[llvm-commits] [llvm] r108219 - /llvm/trunk/lib/CodeGen/TargetInstrInfoImpl.cpp

[Daniel Dunbar]

Hi Jakob,

memcpy-inline.ll is currently failing on llvm-gcc-powerpc-darwin9, like this:
--
******************** TEST 'LLVM :: CodeGen/ARM/memcpy-inline.ll'
FAILED ********************Script:
--
llc < /Users/buildslave/zorg/buildbot/smooshlab/slave/build.llvm-gcc-powerpc-darwin9/llvm.src/test/CodeGen/ARM/memcpy-inline.ll
-mtriple=arm-apple-darwin | grep ldmia
llc < /Users/buildslave/zorg/buildbot/smooshlab/slave/build.llvm-gcc-powerpc-darwin9/llvm.src/test/CodeGen/ARM/memcpy-inline.ll
-mtriple=arm-apple-darwin | grep stmia
llc < /Users/buildslave/zorg/buildbot/smooshlab/slave/build.llvm-gcc-powerpc-darwin9/llvm.src/test/CodeGen/ARM/memcpy-inline.ll
-mtriple=arm-apple-darwin | grep ldrb
llc < /Users/buildslave/zorg/buildbot/smooshlab/slave/build.llvm-gcc-powerpc-darwin9/llvm.src/test/CodeGen/ARM/memcpy-inline.ll
-mtriple=arm-apple-darwin | grep ldrh
--
Exit Code: 1
Command Output (stdout):
--
Command has output on stderr!

--
Command Output (stderr):
--
0  llc               0x00b890cc llvm::sys::RunInterruptHandlers() + 1196
1  llc               0x00b897d8 llvm::sys::PrintStackTraceOnErrorSignal() + 1128
2  libSystem.B.dylib 0x966089fc _sigtramp + 68
Stack dump:
0.	Program arguments: llc -mtriple=arm-apple-darwin
1.	Running pass 'Function Pass Manager' on module '<stdin>'.
2.	Running pass 'ARM Instruction Selection' on function '@t'
--

********************
--

It started in this build:
  http://smooshlab.apple.com:8010/builders/llvm-gcc-powerpc-darwin9/builds/3331
and I think this change is the most likely culprit. Can you take a
look, or point the finger?

 - Daniel

On Mon, Jul 12, 2010 at 5:23 PM, Jakob Stoklund Olesen <stoklund at 2pi.dk> wrote:
> Author: stoklund
> Date: Mon Jul 12 19:23:30 2010
> New Revision: 108219
>
> URL: http://llvm.org/viewvc/llvm-project?rev=108219&view=rev
> Log:
> Don't add memory operands to storeRegToStackSlot / loadRegFromStackSlot results,
> they already have one.
>
> This fixes the himenobmtxpa miscompilation on ARM.
>
> The PostRA scheduler got confused by the double memoperand and hoisted a stack
> slot load above a store to the same slot.
>
> Modified:
>    llvm/trunk/lib/CodeGen/TargetInstrInfoImpl.cpp
>
> Modified: llvm/trunk/lib/CodeGen/TargetInstrInfoImpl.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/TargetInstrInfoImpl.cpp?rev=108219&r1=108218&r2=108219&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/TargetInstrInfoImpl.cpp (original)
> +++ llvm/trunk/lib/CodeGen/TargetInstrInfoImpl.cpp Mon Jul 12 19:23:30 2010
> @@ -24,6 +24,7 @@
>  #include "llvm/CodeGen/MachineRegisterInfo.h"
>  #include "llvm/CodeGen/PostRAHazardRecognizer.h"
>  #include "llvm/CodeGen/PseudoSourceValue.h"
> +#include "llvm/Support/Debug.h"
>  #include "llvm/Support/ErrorHandling.h"
>  #include "llvm/Support/raw_ostream.h"
>  using namespace llvm;
> @@ -253,51 +254,44 @@
>   MachineFunction &MF = *MBB->getParent();
>
>   // Ask the target to do the actual folding.
> -  MachineInstr *NewMI = foldMemoryOperandImpl(MF, MI, Ops, FI);
> +  if (MachineInstr *NewMI = foldMemoryOperandImpl(MF, MI, Ops, FI)) {
> +    // Add a memory operand, foldMemoryOperandImpl doesn't do that.
> +    assert((!(Flags & MachineMemOperand::MOStore) ||
> +            NewMI->getDesc().mayStore()) &&
> +           "Folded a def to a non-store!");
> +    assert((!(Flags & MachineMemOperand::MOLoad) ||
> +            NewMI->getDesc().mayLoad()) &&
> +           "Folded a use to a non-load!");
> +    const MachineFrameInfo &MFI = *MF.getFrameInfo();
> +    assert(MFI.getObjectOffset(FI) != -1);
> +    MachineMemOperand *MMO =
> +      MF.getMachineMemOperand(PseudoSourceValue::getFixedStack(FI),
> +                              Flags, /*Offset=*/0,
> +                              MFI.getObjectSize(FI),
> +                              MFI.getObjectAlignment(FI));
> +    NewMI->addMemOperand(MF, MMO);
>
> -  // Straight COPY may fold as load/store.
> -  if (!NewMI) {
> -    if (!MI->isCopy() || Ops.size() != 1)
> -      return 0;
> -
> -    const TargetRegisterClass *RC = canFoldCopy(MI, Ops[0]);
> -    if (!RC)
> -      return 0;
> -
> -    const MachineOperand &MO = MI->getOperand(1-Ops[0]);
> -    MachineBasicBlock::iterator Pos = MI;
> -    const TargetRegisterInfo *TRI = MF.getTarget().getRegisterInfo();
> -
> -    if (Flags == MachineMemOperand::MOStore)
> -      storeRegToStackSlot(*MBB, Pos, MO.getReg(), MO.isKill(), FI, RC, TRI);
> -    else
> -      loadRegFromStackSlot(*MBB, Pos, MO.getReg(), FI, RC, TRI);
> -
> -    NewMI = --Pos;
> -  } else {
>     // FIXME: change foldMemoryOperandImpl semantics to also insert NewMI.
> -    NewMI = MBB->insert(MI, NewMI);
> +    return MBB->insert(MI, NewMI);
>   }
>
> -  if (!NewMI) return 0;
> -
> +  // Straight COPY may fold as load/store.
> +  if (!MI->isCopy() || Ops.size() != 1)
> +    return 0;
>
> -  assert((!(Flags & MachineMemOperand::MOStore) ||
> -          NewMI->getDesc().mayStore()) &&
> -         "Folded a def to a non-store!");
> -  assert((!(Flags & MachineMemOperand::MOLoad) ||
> -          NewMI->getDesc().mayLoad()) &&
> -         "Folded a use to a non-load!");
> -  const MachineFrameInfo &MFI = *MF.getFrameInfo();
> -  assert(MFI.getObjectOffset(FI) != -1);
> -  MachineMemOperand *MMO =
> -    MF.getMachineMemOperand(PseudoSourceValue::getFixedStack(FI),
> -                            Flags, /*Offset=*/0,
> -                            MFI.getObjectSize(FI),
> -                            MFI.getObjectAlignment(FI));
> -  NewMI->addMemOperand(MF, MMO);
> +  const TargetRegisterClass *RC = canFoldCopy(MI, Ops[0]);
> +  if (!RC)
> +    return 0;
>
> -  return NewMI;
> +  const MachineOperand &MO = MI->getOperand(1-Ops[0]);
> +  MachineBasicBlock::iterator Pos = MI;
> +  const TargetRegisterInfo *TRI = MF.getTarget().getRegisterInfo();
> +
> +  if (Flags == MachineMemOperand::MOStore)
> +    storeRegToStackSlot(*MBB, Pos, MO.getReg(), MO.isKill(), FI, RC, TRI);
> +  else
> +    loadRegFromStackSlot(*MBB, Pos, MO.getReg(), FI, RC, TRI);
> +  return --Pos;
>  }
>
>  /// foldMemoryOperand - Same as the previous version except it allows folding
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>