[llvm] r356396 - Revert r356304: remove subreg parameter from MachineIRBuilder::buildCopy()

Galina Kistanova via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 18 14:23:15 PDT 2019


Hello Amara,

This commit added broken tests to the builder:
http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/16459
. . .
Failing Tests (9):
    LLVM :: CodeGen/AArch64/GlobalISel/fp16-copy-gpr.mir
    LLVM :: CodeGen/AArch64/GlobalISel/select-extract-vector-elt.mir
    LLVM :: CodeGen/AArch64/GlobalISel/select-insert-extract.mir
    LLVM :: CodeGen/AArch64/GlobalISel/select-shuffle-vector.mir
    LLVM :: CodeGen/AArch64/GlobalISel/select-unmerge.mir
    LLVM :: CodeGen/AArch64/arm64-rev.ll
    LLVM :: CodeGen/AArch64/arm64-vfloatintrinsics.ll
    LLVM :: CodeGen/AArch64/f16-instructions.ll
    LLVM :: CodeGen/AArch64/sincospow-vector-expansion.ll

Please have a look?
The builder was already red and did not send any notifications.

Thanks

Galina

On Mon, Mar 18, 2019 at 12:18 PM Amara Emerson via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: aemerson
> Date: Mon Mar 18 12:20:10 2019
> New Revision: 356396
>
> URL: http://llvm.org/viewvc/llvm-project?rev=356396&view=rev
> Log:
> Revert r356304: remove subreg parameter from MachineIRBuilder::buildCopy()
>
> After review comments, it was preferred to not teach MachineIRBuilder about
> non-generic instructions beyond using buildInstr().
>
> For AArch64 I've changed the buildCopy() calls to buildInstr() + a
> separate addReg() call.
>
> This also relaxes the MachineIRBuilder's COPY checking more because it may
> not always have a SrcOp given to it.
>
> Modified:
>     llvm/trunk/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h
>     llvm/trunk/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp
>     llvm/trunk/lib/Target/AArch64/AArch64InstructionSelector.cpp
>
> Modified: llvm/trunk/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h?rev=356396&r1=356395&r2=356396&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h
> (original)
> +++ llvm/trunk/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h Mon Mar
> 18 12:20:10 2019
> @@ -640,8 +640,7 @@ public:
>    /// \pre setBasicBlock or setMI must have been called.
>    ///
>    /// \return a MachineInstrBuilder for the newly created instruction.
> -  MachineInstrBuilder buildCopy(const DstOp &Res, const SrcOp &Op,
> -                                unsigned Subreg = 0);
> +  MachineInstrBuilder buildCopy(const DstOp &Res, const SrcOp &Op);
>
>    /// Build and insert `Res = G_LOAD Addr, MMO`.
>    ///
>
> Modified: llvm/trunk/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp?rev=356396&r1=356395&r2=356396&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp (original)
> +++ llvm/trunk/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp Mon Mar 18
> 12:20:10 2019
> @@ -242,11 +242,8 @@ MachineInstrBuilder MachineIRBuilder::bu
>  }
>
>  MachineInstrBuilder MachineIRBuilder::buildCopy(const DstOp &Res,
> -                                                const SrcOp &Op,
> -                                                unsigned Subreg) {
> -  auto Copy = buildInstr(TargetOpcode::COPY, Res, Op);
> -  Copy->getOperand(1).setSubReg(Subreg);
> -  return Copy;
> +                                                const SrcOp &Op) {
> +  return buildInstr(TargetOpcode::COPY, Res, Op);
>  }
>
>  MachineInstrBuilder MachineIRBuilder::buildConstant(const DstOp &Res,
> @@ -916,6 +913,9 @@ MachineInstrBuilder MachineIRBuilder::bu
>    case TargetOpcode::COPY:
>      assert(DstOps.size() == 1 && "Invalid Dst");
>      assert(SrcOps.size() == 1 && "Invalid Srcs");
> +    assert(DstOps[0].getLLTTy(*getMRI()) == LLT() ||
> +           SrcOps[0].getLLTTy(*getMRI()) == LLT() ||
> +           DstOps[0].getLLTTy(*getMRI()) ==
> SrcOps[0].getLLTTy(*getMRI()));
>      break;
>    case TargetOpcode::G_FCMP:
>    case TargetOpcode::G_ICMP: {
>
> Modified: llvm/trunk/lib/Target/AArch64/AArch64InstructionSelector.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64InstructionSelector.cpp?rev=356396&r1=356395&r2=356396&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Target/AArch64/AArch64InstructionSelector.cpp (original)
> +++ llvm/trunk/lib/Target/AArch64/AArch64InstructionSelector.cpp Mon Mar
> 18 12:20:10 2019
> @@ -478,7 +478,8 @@ static bool selectSubregisterCopy(Machin
>                                    unsigned SubReg) {
>    MachineIRBuilder MIB(I);
>    auto Copy = MIB.buildCopy({From}, {SrcReg});
> -  auto SubRegCopy = MIB.buildCopy({To}, {Copy}, SubReg);
> +  auto SubRegCopy = MIB.buildInstr(TargetOpcode::COPY, {To}, {})
> +                        .addReg(Copy.getReg(0), 0, SubReg);
>    MachineOperand &RegOp = I.getOperand(1);
>    RegOp.setReg(SubRegCopy.getReg(0));
>
> @@ -1104,7 +1105,8 @@ bool AArch64InstructionSelector::select(
>
>      unsigned DstReg = MRI.createGenericVirtualRegister(LLT::scalar(64));
>      MIB.setInsertPt(MIB.getMBB(), std::next(I.getIterator()));
> -    MIB.buildCopy({I.getOperand(0).getReg()}, {DstReg}, AArch64::sub_32);
> +    MIB.buildInstr(TargetOpcode::COPY, {I.getOperand(0).getReg()}, {})
> +        .addReg(DstReg, 0, AArch64::sub_32);
>      RBI.constrainGenericRegister(I.getOperand(0).getReg(),
>                                   AArch64::GPR32RegClass, MRI);
>      I.getOperand(0).setReg(DstReg);
> @@ -1938,7 +1940,8 @@ MachineInstr *AArch64InstructionSelector
>      DstReg = MRI.createVirtualRegister(DstRC);
>    // If the lane index is 0, we just use a subregister COPY.
>    if (LaneIdx == 0) {
> -    auto Copy = MIRBuilder.buildCopy({*DstReg}, {VecReg}, ExtractSubReg);
> +    auto Copy = MIRBuilder.buildInstr(TargetOpcode::COPY, {*DstReg}, {})
> +                    .addReg(VecReg, 0, ExtractSubReg);
>      RBI.constrainGenericRegister(*DstReg, *DstRC, MRI);
>      return &*Copy;
>    }
> @@ -2115,7 +2118,8 @@ bool AArch64InstructionSelector::selectU
>    //
>    // Perform the first copy separately as a subregister copy.
>    unsigned CopyTo = I.getOperand(0).getReg();
> -  auto FirstCopy = MIB.buildCopy({CopyTo}, {InsertRegs[0]},
> ExtractSubReg);
> +  auto FirstCopy = MIB.buildInstr(TargetOpcode::COPY, {CopyTo}, {})
> +                       .addReg(InsertRegs[0], 0, ExtractSubReg);
>    constrainSelectedInstRegOperands(*FirstCopy, TII, TRI, RBI);
>
>    // Now, perform the remaining copies as vector lane copies.
> @@ -2388,7 +2392,9 @@ bool AArch64InstructionSelector::selectS
>      constrainSelectedInstRegOperands(*TBL1, TII, TRI, RBI);
>
>      auto Copy =
> -        MIRBuilder.buildCopy({I.getOperand(0).getReg()}, {TBL1},
> AArch64::dsub);
> +        MIRBuilder
> +            .buildInstr(TargetOpcode::COPY, {I.getOperand(0).getReg()},
> {})
> +            .addReg(TBL1.getReg(0), 0, AArch64::dsub);
>      RBI.constrainGenericRegister(Copy.getReg(0), AArch64::FPR64RegClass,
> MRI);
>      I.eraseFromParent();
>      return true;
> @@ -2538,7 +2544,8 @@ bool AArch64InstructionSelector::selectB
>      unsigned Reg = MRI.createVirtualRegister(RC);
>      unsigned DstReg = I.getOperand(0).getReg();
>
> -    MIRBuilder.buildCopy({DstReg}, {DstVec}, SubReg);
> +    MIRBuilder.buildInstr(TargetOpcode::COPY, {DstReg}, {})
> +        .addReg(DstVec, 0, SubReg);
>      MachineOperand &RegOp = I.getOperand(1);
>      RegOp.setReg(Reg);
>      RBI.constrainGenericRegister(DstReg, *RC, MRI);
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190318/fd9d65cc/attachment.html>


More information about the llvm-commits mailing list