[llvm] r260260 - [AArch64] Remove redundant calls and clang format. NFC.

Chad Rosier via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 30 11:15:33 PDT 2016


Should be fixed in r264881.  Thanks, Hal.

-----Original Message-----
From: Hal Finkel [mailto:hfinkel at anl.gov] 
Sent: Saturday, March 26, 2016 10:29 AM
To: Chad Rosier <mcrosier at codeaurora.org>
Cc: llvm-commits at lists.llvm.org
Subject: Re: [llvm] r260260 - [AArch64] Remove redundant calls and clang format. NFC.

----- Original Message -----
> From: "Chad Rosier via llvm-commits" <llvm-commits at lists.llvm.org>
> To: llvm-commits at lists.llvm.org
> Sent: Tuesday, February 9, 2016 1:33:43 PM
> Subject: [llvm] r260260 - [AArch64] Remove redundant calls and clang format. NFC.
> 
> Author: mcrosier
> Date: Tue Feb  9 13:33:42 2016
> New Revision: 260260
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=260260&view=rev
> Log:
> [AArch64] Remove redundant calls and clang format. NFC.
> 
> Modified:
>     llvm/trunk/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp
> 
> Modified: llvm/trunk/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp?rev=260260&r1=260259&r2=260260&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp
> (original)
> +++ llvm/trunk/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp Tue
> Feb  9 13:33:42 2016
> @@ -680,6 +680,8 @@ AArch64LoadStoreOpt::mergeNarrowInsns(Ma
>      OffsetImm /= 2;
>    }
>  
> +  DebugLoc DL = I->getDebugLoc();
> +  MachineBasicBlock *MBB = I->getParent();
>    if (isNarrowLoad(Opc)) {
>      MachineInstr *RtNewDest = MergeForward ? I : MergeMI;
>      // When merging small (< 32 bit) loads for big-endian targets,
>      the order of
> @@ -688,12 +690,12 @@ AArch64LoadStoreOpt::mergeNarrowInsns(Ma
>        std::swap(RtMI, Rt2MI);
>      // Construct the new load instruction.
>      MachineInstr *NewMemMI, *BitExtMI1, *BitExtMI2;

FWIW, GCC now warns here:

/src/llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp: In member function ‘llvm::MachineBasicBlock::iterator {anonymous}::AArch64LoadStoreOpt::mergeNarrowInsns(llvm::MachineBasicBlock::iterator, llvm::MachineBasicBlock::iterator, const LdStPairFlags&)’:
/src/llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp:674:19: warning: variable ‘NewMemMI’ set but not used [-Wunused-but-set-variable]
     MachineInstr *NewMemMI, *BitExtMI1, *BitExtMI2;
                   ^
/src/llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp:674:30: warning: variable ‘BitExtMI1’ set but not used [-Wunused-but-set-variable]
     MachineInstr *NewMemMI, *BitExtMI1, *BitExtMI2;
                              ^
/src/llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp:674:42: warning: variable ‘BitExtMI2’ set but not used [-Wunused-but-set-variable]
     MachineInstr *NewMemMI, *BitExtMI1, *BitExtMI2;
                                          ^
/src/llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp: In member function ‘llvm::MachineBasicBlock::iterator {anonymous}::AArch64LoadStoreOpt::promoteLoadFromStore(llvm::MachineBasicBlock::iterator, llvm::MachineBasicBlock::iterator)’:
/src/llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp:930:17: warning: variable ‘BitExtMI’ set but not used [-Wunused-but-set-variable]
   MachineInstr *BitExtMI;
                 ^

 -Hal

> -    NewMemMI = BuildMI(*I->getParent(), InsertionPoint,
> I->getDebugLoc(),
> -                       TII->get(getMatchingWideOpcode(Opc)))
> -                   .addOperand(getLdStRegOp(RtNewDest))
> -                   .addOperand(BaseRegOp)
> -                   .addImm(OffsetImm)
> -                   .setMemRefs(I->mergeMemRefsWith(*MergeMI));
> +    NewMemMI =
> +        BuildMI(*MBB, InsertionPoint, DL,
> TII->get(getMatchingWideOpcode(Opc)))
> +            .addOperand(getLdStRegOp(RtNewDest))
> +            .addOperand(BaseRegOp)
> +            .addImm(OffsetImm)
> +            .setMemRefs(I->mergeMemRefsWith(*MergeMI));
>  
>      DEBUG(
>          dbgs()
> @@ -712,53 +714,51 @@ AArch64LoadStoreOpt::mergeNarrowInsns(Ma
>      MachineInstr *ExtDestMI = MergeForward ? MergeMI : I;
>      if ((ExtDestMI == Rt2MI) == Subtarget->isLittleEndian()) {
>        // Create the bitfield extract for high bits.
> -      BitExtMI1 = BuildMI(*I->getParent(), InsertionPoint,
> I->getDebugLoc(),
> -                          TII->get(getBitExtrOpcode(Rt2MI)))
> -                      .addOperand(getLdStRegOp(Rt2MI))
> -                      .addReg(getLdStRegOp(RtNewDest).getReg())
> -                      .addImm(LSBHigh)
> -                      .addImm(ImmsHigh);
> +      BitExtMI1 =
> +          BuildMI(*MBB, InsertionPoint, DL,
> TII->get(getBitExtrOpcode(Rt2MI)))
> +              .addOperand(getLdStRegOp(Rt2MI))
> +              .addReg(getLdStRegOp(RtNewDest).getReg())
> +              .addImm(LSBHigh)
> +              .addImm(ImmsHigh);
>        // Create the bitfield extract for low bits.
>        if (RtMI->getOpcode() ==
>        getMatchingNonSExtOpcode(RtMI->getOpcode())) {
>          // For unsigned, prefer to use AND for low bits.
> -        BitExtMI2 = BuildMI(*I->getParent(), InsertionPoint,
> I->getDebugLoc(),
> -                            TII->get(AArch64::ANDWri))
> +        BitExtMI2 = BuildMI(*MBB, InsertionPoint, DL,
> TII->get(AArch64::ANDWri))
>                          .addOperand(getLdStRegOp(RtMI))
>                          .addReg(getLdStRegOp(RtNewDest).getReg())
>                          .addImm(ImmsLow);
>        } else {
> -        BitExtMI2 = BuildMI(*I->getParent(), InsertionPoint,
> I->getDebugLoc(),
> -                            TII->get(getBitExtrOpcode(RtMI)))
> -                        .addOperand(getLdStRegOp(RtMI))
> -                        .addReg(getLdStRegOp(RtNewDest).getReg())
> -                        .addImm(LSBLow)
> -                        .addImm(ImmsLow);
> +        BitExtMI2 =
> +            BuildMI(*MBB, InsertionPoint, DL,
> TII->get(getBitExtrOpcode(RtMI)))
> +                .addOperand(getLdStRegOp(RtMI))
> +                .addReg(getLdStRegOp(RtNewDest).getReg())
> +                .addImm(LSBLow)
> +                .addImm(ImmsLow);
>        }
>      } else {
>        // Create the bitfield extract for low bits.
>        if (RtMI->getOpcode() ==
>        getMatchingNonSExtOpcode(RtMI->getOpcode())) {
>          // For unsigned, prefer to use AND for low bits.
> -        BitExtMI1 = BuildMI(*I->getParent(), InsertionPoint,
> I->getDebugLoc(),
> -                            TII->get(AArch64::ANDWri))
> +        BitExtMI1 = BuildMI(*MBB, InsertionPoint, DL,
> TII->get(AArch64::ANDWri))
>                          .addOperand(getLdStRegOp(RtMI))
>                          .addReg(getLdStRegOp(RtNewDest).getReg())
>                          .addImm(ImmsLow);
>        } else {
> -        BitExtMI1 = BuildMI(*I->getParent(), InsertionPoint,
> I->getDebugLoc(),
> -                            TII->get(getBitExtrOpcode(RtMI)))
> -                        .addOperand(getLdStRegOp(RtMI))
> -                        .addReg(getLdStRegOp(RtNewDest).getReg())
> -                        .addImm(LSBLow)
> -                        .addImm(ImmsLow);
> +        BitExtMI1 =
> +            BuildMI(*MBB, InsertionPoint, DL,
> TII->get(getBitExtrOpcode(RtMI)))
> +                .addOperand(getLdStRegOp(RtMI))
> +                .addReg(getLdStRegOp(RtNewDest).getReg())
> +                .addImm(LSBLow)
> +                .addImm(ImmsLow);
>        }
>  
>        // Create the bitfield extract for high bits.
> -      BitExtMI2 = BuildMI(*I->getParent(), InsertionPoint,
> I->getDebugLoc(),
> -                          TII->get(getBitExtrOpcode(Rt2MI)))
> -                      .addOperand(getLdStRegOp(Rt2MI))
> -                      .addReg(getLdStRegOp(RtNewDest).getReg())
> -                      .addImm(LSBHigh)
> -                      .addImm(ImmsHigh);
> +      BitExtMI2 =
> +          BuildMI(*MBB, InsertionPoint, DL,
> TII->get(getBitExtrOpcode(Rt2MI)))
> +              .addOperand(getLdStRegOp(Rt2MI))
> +              .addReg(getLdStRegOp(RtNewDest).getReg())
> +              .addImm(LSBHigh)
> +              .addImm(ImmsHigh);
>      }
>      DEBUG(dbgs() << "    ");
>      DEBUG((BitExtMI1)->print(dbgs()));
> @@ -775,8 +775,7 @@ AArch64LoadStoreOpt::mergeNarrowInsns(Ma
>  
>    // Construct the new instruction.
>    MachineInstrBuilder MIB;
> -  MIB = BuildMI(*I->getParent(), InsertionPoint, I->getDebugLoc(),
> -                TII->get(getMatchingWideOpcode(Opc)))
> +  MIB = BuildMI(*MBB, InsertionPoint, DL,
> TII->get(getMatchingWideOpcode(Opc)))
>              .addOperand(getLdStRegOp(I))
>              .addOperand(BaseRegOp)
>              .addImm(OffsetImm)
> @@ -848,8 +847,9 @@ AArch64LoadStoreOpt::mergePairedInsns(Ma
>  
>    // Construct the new instruction.
>    MachineInstrBuilder MIB;
> -  MIB = BuildMI(*I->getParent(), InsertionPoint, I->getDebugLoc(),
> -                TII->get(getMatchingPairOpcode(Opc)))
> +  DebugLoc DL = I->getDebugLoc();
> +  MachineBasicBlock *MBB = I->getParent();
> +  MIB = BuildMI(*MBB, InsertionPoint, DL,
> TII->get(getMatchingPairOpcode(Opc)))
>              .addOperand(getLdStRegOp(RtMI))
>              .addOperand(getLdStRegOp(Rt2MI))
>              .addOperand(BaseRegOp)
> @@ -885,15 +885,13 @@ AArch64LoadStoreOpt::mergePairedInsns(Ma
>      // Insert this definition right after the generated LDP, i.e.,
>      before
>      // InsertionPoint.
>      MachineInstrBuilder MIBKill =
> -        BuildMI(*I->getParent(), InsertionPoint, I->getDebugLoc(),
> -                TII->get(TargetOpcode::KILL), DstRegW)
> +        BuildMI(*MBB, InsertionPoint, DL,
> TII->get(TargetOpcode::KILL), DstRegW)
>              .addReg(DstRegW)
>              .addReg(DstRegX, RegState::Define);
>      MIBKill->getOperand(2).setImplicit();
>      // Create the sign extension.
>      MachineInstrBuilder MIBSXTW =
> -        BuildMI(*I->getParent(), InsertionPoint, I->getDebugLoc(),
> -                TII->get(AArch64::SBFMXri), DstRegX)
> +        BuildMI(*MBB, InsertionPoint, DL,
> TII->get(AArch64::SBFMXri), DstRegX)
>              .addReg(DstRegX)
>              .addImm(0)
>              .addImm(31);
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory



More information about the llvm-commits mailing list