[llvm] r215155 - fix materialization of one bit constants and global values which are accessed through

Eric Christopher echristo at gmail.com
Mon Aug 11 23:54:24 PDT 2014 

On Mon, Aug 11, 2014 at 2:32 AM, Daniel Sanders
<Daniel.Sanders at imgtec.com> wrote:
> Hi Eric,
>
> I could be wrong, but I don't think it's possible to write a robust test case for the boolean materialization bug. My thinking on reading the patch was that the bug would occur when FastISel generates the constant as an i1, but SelectionDAG eventually consumes it expecting an target boolean (i32 zero extended from an i1[*]). As FastISel handles more of the IR, the places where this can occur will disappear.
>

Seemed there was likely a testcase that the code came from of it came
from though? Something reduced that highlighted the problem for the
patch?

-eric

> I suppose we could have a simple test for 'ldi 1' rather than 'ldi -1', however both are correct according to the IR and it follows that a more complete FastISel implementation should be able to use either. Having said that, I've just looked at the boundary between FastISel and SelectionDAG (in SelectionDAGISel::SelectAllBasicBlocks()) and I couldn't see anything that legalizes the types/values that FastISel used so perhaps there's an unwritten rule that says that FastISel must legalize in a similar way to SelectionDAG.
>
> [*] Sign-extended for MIPS32r6/MIPS64r6 floating-point comparisons but we can handle that later.
>
>> -----Original Message-----
>> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
>> bounces at cs.uiuc.edu] On Behalf Of Eric Christopher
>> Sent: 08 August 2014 23:20
>> To: Reed Kotler
>> Cc: llvm-commits at cs.uiuc.edu
>> Subject: Re: [llvm] r215155 - fix materialization of one bit constants and global
>> values which are accessed through
>>
>> Testcase for the boolean materialization?
>>
>> -eric
>>
>> On Thu, Aug 7, 2014 at 3:09 PM, Reed Kotler <rkotler at mips.com> wrote:
>> > Author: rkotler
>> > Date: Thu Aug  7 17:09:01 2014
>> > New Revision: 215155
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=215155&view=rev
>> > Log:
>> > fix materialization of one bit constants and global values which are accessed
>> through
>> > a base GOT entry.
>> >
>> > Summary:
>> > get tip of tree mips fast-isel to pass test-suite
>> >
>> > Two bugs were fixed:
>> >
>> > 1) one bit booleans were treated as 1 bit signed integers and so the literal
>> '1' could become sign extended.
>> > 2) mips uses got for pic but in certain cases, as with string constants for
>> example, many items can be referenced from the same got entry and this
>> case was not handled properly.
>> >
>> > Test Plan: test-suite
>> >
>> > Reviewers: dsanders
>> >
>> > Reviewed By: dsanders
>> >
>> > Subscribers: mcrosier
>> >
>> > Differential Revision: http://reviews.llvm.org/D4801
>> >
>> > Added:
>> >     llvm/trunk/test/CodeGen/Mips/Fast-ISel/loadstrconst.ll
>> > Modified:
>> >     llvm/trunk/lib/Target/Mips/MipsFastISel.cpp
>> >
>> > Modified: llvm/trunk/lib/Target/Mips/MipsFastISel.cpp
>> > URL: http://llvm.org/viewvc/llvm-
>> project/llvm/trunk/lib/Target/Mips/MipsFastISel.cpp?rev=215155&r1=21515
>> 4&r2=215155&view=diff
>> >
>> ==========================================================
>> ====================
>> > --- llvm/trunk/lib/Target/Mips/MipsFastISel.cpp (original)
>> > +++ llvm/trunk/lib/Target/Mips/MipsFastISel.cpp Thu Aug  7 17:09:01 2014
>> > @@ -110,7 +110,7 @@ private:
>> >    }
>> >
>> >    MachineInstrBuilder EmitInstLoad(unsigned Opc, unsigned DstReg,
>> > -                                      unsigned MemReg, int64_t MemOffset) {
>> > +                                   unsigned MemReg, int64_t MemOffset) {
>> >      return EmitInst(Opc, DstReg).addReg(MemReg).addImm(MemOffset);
>> >    }
>> >
>> > @@ -353,15 +353,23 @@ unsigned MipsFastISel::MaterializeGV(con
>> >      return 0;
>> >    EmitInst(Mips::LW, DestReg).addReg(MFI-
>> >getGlobalBaseReg()).addGlobalAddress(
>> >        GV, 0, MipsII::MO_GOT);
>> > +  if ((GV->hasInternalLinkage() ||
>> > +       (GV->hasLocalLinkage() && !isa<Function>(GV)))) {
>> > +    unsigned TempReg = createResultReg(RC);
>> > +    EmitInst(Mips::ADDiu, TempReg).addReg(DestReg).addGlobalAddress(
>> > +        GV, 0, MipsII::MO_ABS_LO);
>> > +    DestReg = TempReg;
>> > +  }
>> >    return DestReg;
>> >  }
>> > +
>> >  unsigned MipsFastISel::MaterializeInt(const Constant *C, MVT VT) {
>> >    if (VT != MVT::i32 && VT != MVT::i16 && VT != MVT::i8 && VT != MVT::i1)
>> >      return 0;
>> >    const TargetRegisterClass *RC = &Mips::GPR32RegClass;
>> >    const ConstantInt *CI = cast<ConstantInt>(C);
>> >    int64_t Imm;
>> > -  if (CI->isNegative())
>> > +  if ((VT != MVT::i1) && CI->isNegative())
>> >      Imm = CI->getSExtValue();
>> >    else
>> >      Imm = CI->getZExtValue();
>> >
>> > Added: llvm/trunk/test/CodeGen/Mips/Fast-ISel/loadstrconst.ll
>> > URL: http://llvm.org/viewvc/llvm-
>> project/llvm/trunk/test/CodeGen/Mips/Fast-
>> ISel/loadstrconst.ll?rev=215155&view=auto
>> >
>> ==========================================================
>> ====================
>> > --- llvm/trunk/test/CodeGen/Mips/Fast-ISel/loadstrconst.ll (added)
>> > +++ llvm/trunk/test/CodeGen/Mips/Fast-ISel/loadstrconst.ll Thu Aug  7
>> 17:09:01 2014
>> > @@ -0,0 +1,19 @@
>> > +; RUN: llc -march=mipsel -relocation-model=pic -O0 -mips-fast-isel -fast-
>> isel-abort -mcpu=mips32r2 \
>> > +; RUN:     < %s | FileCheck %s
>> > +
>> > + at .str = private unnamed_addr constant [6 x i8] c"hello\00", align 1
>> > + at s = common global i8* null, align 4
>> > +
>> > +; Function Attrs: nounwind
>> > +define void @foo() #0 {
>> > +entry:
>> > +  store i8* getelementptr inbounds ([6 x i8]* @.str, i32 0, i32 0), i8** @s,
>> align 4
>> > +  ret void
>> > +; CHECK:        .ent    foo
>> > +; CHECK:        lw      $[[REG1:[0-9]+]], %got($.str)(${{[0-9]+}})
>> > +; CHECK:        addiu   ${{[0-9]+}}, $[[REG1]], %lo($.str)
>> > +
>> > +}
>> > +
>> > +attributes #0 = { nounwind "less-precise-fpmad"="false" "no-frame-
>> pointer-elim"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false"
>> "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-
>> float"="false" }
>> > +
>> >
>> >
>> > _______________________________________________
>> > llvm-commits mailing list
>> > llvm-commits at cs.uiuc.edu
>> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

More information about the llvm-commits mailing list