[llvm] r305455 - [mips][microMIPS] Extending size reduction pass with ADDIUSP and ADDIUR1SP

Simon Dardis via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 16 07:06:24 PDT 2017


Reverted in r305557.

Simon

From: llvm-commits [mailto:llvm-commits-bounces at lists.llvm.org] On Behalf Of Kostya Serebryany via llvm-commits
Sent: 15 June 2017 18:40
To: Zoran Jovanovic
Cc: LLVM Commits
Subject: Re: [llvm] r305455 - [mips][microMIPS] Extending size reduction pass with ADDIUSP and ADDIUR1SP

This or related patch breaks ubsan bots. 
Please fix ASAP.
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/5691/steps/check-llvm%20ubsan/logs/stdio
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/Mips/MicroMipsSizeReduction.cpp:273:15: runtime error: left shift of negative value -2
    #0 0x15bfbf5 in InRange(long, unsigned short, int, int) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/Mips/MicroMipsSizeReduction.cpp:273:15
    #1 0x15bf529 in ImmInRange(llvm::MachineInstr*, (anonymous namespace)::ReduceEntry const&) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/Mips/MicroMipsSizeReduction.cpp:286:8
    #2 0x15beddd in (anonymous namespace)::MicroMipsSizeReduce::ReduceADDIUToADDIUR1SP(llvm::MachineInstr*, (anonymous namespace)::ReduceEntry const&) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/Mips/MicroMipsSizeReduction.cpp:347:8
    #3 0x15c027a in (anonymous namespace)::MicroMipsSizeReduce::ReduceMI(llvm::ilist_iterator<llvm::ilist_detail::node_options<llvm::MachineInstr, true, true, void>, false, false> const&) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/Mips/MicroMipsSizeReduction.cpp:315:9
    #4 0x15c0134 in (anonymous namespace)::MicroMipsSizeReduce::ReduceMBB(llvm::MachineBasicBlock&) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/Mips/MicroMipsSizeReduction.cpp:414:17
    #5 0x15bffe6 in (anonymous namespace)::MicroMipsSizeReduce::runOnMachineFunction(llvm::MachineFunction&) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/Mips/MicroMipsSizeReduction.cpp:470:17
    #6 0x2318d4e in llvm::MachineFunctionPass::runOnFunction(llvm::Function&) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/CodeGen/MachineFunctionPass.cpp:62:13
    #7 0x2814c20 in llvm::FPPassManager::runOnFunction(llvm::Function&) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/IR/LegacyPassManager.cpp:1519:27
    #8 0x2814f86 in llvm::FPPassManager::runOnModule(llvm::Module&) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/IR/LegacyPassManager.cpp:1540:16
    #9 0x281582e in (anonymous namespace)::MPPassManager::runOnModule(llvm::Module&) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/IR/LegacyPassManager.cpp:1596:27
    #10 0x28152f1 in llvm::legacy::PassManagerImpl::run(llvm::Module&) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/IR/LegacyPassManager.cpp:1699:44
    #11 0xa1513a in compileModule(char**, llvm::LLVMContext&) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/llc/llc.cpp:609:8
    #12 0xa139da in main /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/llc/llc.cpp:350:22
    #13 0x7f7f34f6b82f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #14 0x9ec2a8 in _start (/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm_build_ubsan/bin/llc+0x9ec2a8)

On Thu, Jun 15, 2017 at 2:14 AM, Zoran Jovanovic via llvm-commits <llvm-commits at lists.llvm.org> wrote:
Author: zjovanovic
Date: Thu Jun 15 04:14:33 2017
New Revision: 305455

URL: http://llvm.org/viewvc/llvm-project?rev=305455&view=rev
Log:
[mips][microMIPS] Extending size reduction pass with ADDIUSP and ADDIUR1SP
Author: milena.vujosevic.janicic
Reviewers: sdardis
The patch extends size reduction pass for MicroMIPS.
The following instructions are examined and transformed, if possible:
ADDIU instruction is transformed into 16-bit instruction ADDIUSP
ADDIU instruction is transformed into 16-bit instruction ADDIUR1SP
Differential Revision: https://reviews.llvm.org/D33887

Added:
    llvm/trunk/test/CodeGen/Mips/micromips-sizereduction/micromips-addiur1sp-addiusp.ll
Modified:
    llvm/trunk/lib/Target/Mips/MicroMipsSizeReduction.cpp

Modified: llvm/trunk/lib/Target/Mips/MicroMipsSizeReduction.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/MicroMipsSizeReduction.cpp?rev=305455&r1=305454&r2=305455&view=diff
==============================================================================
--- llvm/trunk/lib/Target/Mips/MicroMipsSizeReduction.cpp (original)
+++ llvm/trunk/lib/Target/Mips/MicroMipsSizeReduction.cpp Thu Jun 15 04:14:33 2017
@@ -32,6 +32,8 @@ namespace {
 enum OperandTransfer {
   OT_NA,          ///< Not applicable
   OT_OperandsAll, ///< Transfer all operands
+  OT_Operands02,  ///< Transfer operands 0 and 2
+  OT_Operand2,    ///< Transfer just operand 2
 };

 /// Reduction type
@@ -143,14 +145,24 @@ private:
   // returns true on success.
   static bool ReduceSXtoSX16(MachineInstr *MI, const ReduceEntry &Entry);

-  // Attempts to reduce arithmetic instructions, returns true on success
+  // Attempts to reduce arithmetic instructions, returns true on success.
   static bool ReduceArithmeticInstructions(MachineInstr *MI,
                                            const ReduceEntry &Entry);

-  // Changes opcode of an instruction
+  // Attempts to reduce ADDIU into ADDIUSP instruction,
+  // returns true on success.
+  static bool ReduceADDIUToADDIUSP(MachineInstr *MI,
+                                   const ReduceEntry &Entry);
+
+  // Attempts to reduce ADDIU into ADDIUR1SP instruction,
+  // returns true on success.
+  static bool ReduceADDIUToADDIUR1SP(MachineInstr *MI,
+                                     const ReduceEntry &Entry);
+
+  // Changes opcode of an instruction.
   static bool ReplaceInstruction(MachineInstr *MI, const ReduceEntry &Entry);

-  // Table with transformation rules for each instruction
+  // Table with transformation rules for each instruction.
   static llvm::SmallVector<ReduceEntry, 16> ReduceTable;
 };

@@ -158,12 +170,20 @@ char MicroMipsSizeReduce::ID = 0;
 const MipsInstrInfo *MicroMipsSizeReduce::MipsII;

 // This table must be sorted by WideOpc as a main criterion and
-// ReduceType as a sub-criterion (when wide opcodes are the same)
+// ReduceType as a sub-criterion (when wide opcodes are the same).
 llvm::SmallVector<ReduceEntry, 16> MicroMipsSizeReduce::ReduceTable = {

     // ReduceType, OpCodes, ReduceFunction,
     // OpInfo(TransferOperands),
     // ImmField(Shift, LBound, HBound, ImmFieldPosition)
+    {RT_OneInstr, OpCodes(Mips::ADDiu, Mips::ADDIUR1SP_MM),
+     ReduceADDIUToADDIUR1SP, OpInfo(OT_Operands02), ImmField(2, 0, 64, 2)},
+    {RT_OneInstr, OpCodes(Mips::ADDiu, Mips::ADDIUSP_MM),
+     ReduceADDIUToADDIUSP, OpInfo(OT_Operand2), ImmField(0, 0, 0, 2)},
+    {RT_OneInstr, OpCodes(Mips::ADDiu_MM, Mips::ADDIUR1SP_MM),
+     ReduceADDIUToADDIUR1SP, OpInfo(OT_Operands02), ImmField(2, 0, 64, 2)},
+    {RT_OneInstr, OpCodes(Mips::ADDiu_MM, Mips::ADDIUSP_MM),
+     ReduceADDIUToADDIUSP, OpInfo(OT_Operand2), ImmField(0, 0, 0, 2)},
     {RT_OneInstr, OpCodes(Mips::ADDu, Mips::ADDU16_MM),
      ReduceArithmeticInstructions, OpInfo(OT_OperandsAll),
      ImmField(0, 0, 0, -1)},
@@ -174,6 +194,8 @@ llvm::SmallVector<ReduceEntry, 16> Micro
      OpInfo(OT_OperandsAll), ImmField(0, -1, 15, 2)},
     {RT_OneInstr, OpCodes(Mips::LBu_MM, Mips::LBU16_MM), ReduceLXUtoLXU16,
      OpInfo(OT_OperandsAll), ImmField(0, -1, 15, 2)},
+    {RT_OneInstr, OpCodes(Mips::LEA_ADDiu, Mips::ADDIUR1SP_MM),
+     ReduceADDIUToADDIUR1SP, OpInfo(OT_Operands02), ImmField(2, 0, 64, 2)},
     {RT_OneInstr, OpCodes(Mips::LHu, Mips::LHU16_MM), ReduceLXUtoLXU16,
      OpInfo(OT_OperandsAll), ImmField(1, 0, 16, 2)},
     {RT_OneInstr, OpCodes(Mips::LHu_MM, Mips::LHU16_MM), ReduceLXUtoLXU16,
@@ -203,7 +225,7 @@ llvm::SmallVector<ReduceEntry, 16> Micro
 };
 }

-// Returns true if the machine operand MO is register SP
+// Returns true if the machine operand MO is register SP.
 static bool IsSP(const MachineOperand &MO) {
   if (MO.isReg() && ((MO.getReg() == Mips::SP)))
     return true;
@@ -225,7 +247,7 @@ static bool isMMSourceRegister(const Mac
 }

 // Returns true if the operand Op is an immediate value
-// and writes the immediate value into variable Imm
+// and writes the immediate value into variable Imm.
 static bool GetImm(MachineInstr *MI, unsigned Op, int64_t &Imm) {

   if (!MI->getOperand(Op).isImm())
@@ -234,8 +256,17 @@ static bool GetImm(MachineInstr *MI, uns
   return true;
 }

+// Returns true if the value is a valid immediate for ADDIUSP.
+static bool AddiuspImmValue(int64_t Value) {
+  int64_t Value2 = Value >> 2;
+  if (Value == (Value2 << 2) &&
+      ((Value2 >= 2 && Value2 <= 257) || (Value2 >= -258 && Value2 <= -3)))
+    return true;
+  return false;
+}
+
 // Returns true if the variable Value has the number of least-significant zero
-// bits equal to Shift and if the shifted value is between the bounds
+// bits equal to Shift and if the shifted value is between the bounds.
 static bool InRange(int64_t Value, unsigned short Shift, int LBound,
                     int HBound) {
   int64_t Value2 = Value >> Shift;
@@ -244,7 +275,7 @@ static bool InRange(int64_t Value, unsig
   return false;
 }

-// Returns true if immediate operand is in range
+// Returns true if immediate operand is in range.
 static bool ImmInRange(MachineInstr *MI, const ReduceEntry &Entry) {

   int64_t offset;
@@ -310,6 +341,34 @@ bool MicroMipsSizeReduce::ReduceArithmet
   return ReplaceInstruction(MI, Entry);
 }

+bool MicroMipsSizeReduce::ReduceADDIUToADDIUR1SP(MachineInstr *MI,
+                                                 const ReduceEntry &Entry) {
+
+  if (!ImmInRange(MI, Entry))
+    return false;
+
+  if (!isMMThreeBitGPRegister(MI->getOperand(0)) || !IsSP(MI->getOperand(1)))
+    return false;
+
+  return ReplaceInstruction(MI, Entry);
+}
+
+bool MicroMipsSizeReduce::ReduceADDIUToADDIUSP(MachineInstr *MI,
+                                               const ReduceEntry &Entry) {
+
+  int64_t ImmValue;
+  if (!GetImm(MI, Entry.ImmField(), ImmValue))
+    return false;
+
+  if (!AddiuspImmValue(ImmValue))
+    return false;
+
+  if (!IsSP(MI->getOperand(0)) || !IsSP(MI->getOperand(1)))
+    return false;
+
+  return ReplaceInstruction(MI, Entry);
+}
+
 bool MicroMipsSizeReduce::ReduceLXUtoLXU16(MachineInstr *MI,
                                            const ReduceEntry &Entry) {

@@ -361,10 +420,36 @@ bool MicroMipsSizeReduce::ReduceMBB(Mach
 bool MicroMipsSizeReduce::ReplaceInstruction(MachineInstr *MI,
                                              const ReduceEntry &Entry) {

-  MI->setDesc(MipsII->get(Entry.NarrowOpc()));
-  DEBUG(dbgs() << "Converted into 16-bit: " << *MI);
-  ++NumReduced;
-  return true;
+  enum OperandTransfer OpTransfer = Entry.TransferOperands();
+  if (OpTransfer == OT_OperandsAll) {
+    DEBUG(dbgs() << "Converted 32-bit: " << *MI);
+    MI->setDesc(MipsII->get(Entry.NarrowOpc()));
+    DEBUG(dbgs() << "       to 16-bit: " << *MI);
+    ++NumReduced;
+    return true;
+  } else {
+    MachineBasicBlock &MBB = *MI->getParent();
+    const MCInstrDesc &NewMCID = MipsII->get(Entry.NarrowOpc());
+    DebugLoc dl = MI->getDebugLoc();
+    MachineInstrBuilder MIB = BuildMI(MBB, MI, dl, NewMCID);
+
+    if (OpTransfer == OT_Operand2)
+      MIB.add(MI->getOperand(2));
+    else if (OpTransfer == OT_Operands02) {
+      MIB.add(MI->getOperand(0));
+      MIB.add(MI->getOperand(2));
+    }
+
+    // Transfer MI flags.
+    MIB.setMIFlags(MI->getFlags());
+
+    DEBUG(dbgs() << "Converted 32-bit: " << *MI
+                 << "       to 16-bit: " << *MIB);
+    MBB.erase_instr(MI);
+    ++NumReduced;
+    return true;
+  }
+  return false;
 }

 bool MicroMipsSizeReduce::runOnMachineFunction(MachineFunction &MF) {

Added: llvm/trunk/test/CodeGen/Mips/micromips-sizereduction/micromips-addiur1sp-addiusp.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/Mips/micromips-sizereduction/micromips-addiur1sp-addiusp.ll?rev=305455&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/Mips/micromips-sizereduction/micromips-addiur1sp-addiusp.ll (added)
+++ llvm/trunk/test/CodeGen/Mips/micromips-sizereduction/micromips-addiur1sp-addiusp.ll Thu Jun 15 04:14:33 2017
@@ -0,0 +1,17 @@
+; RUN: llc -march=mipsel -mcpu=mips32r2 -mattr=+micromips -verify-machineinstrs < %s | FileCheck %s
+
+define i32 @f1() {
+entry:
+; CHECK-LABEL: f1:
+; CHECK: addiusp
+; CHECK: addiur1sp
+; CHECK: addiusp
+  %a = alloca [10 x i32], align 4
+  %index = getelementptr inbounds [10 x i32], [10 x i32]* %a, i32 0, i32 0
+  call void @init(i32* %index)
+  %0 = load i32, i32* %index, align 4
+  ret i32 %0
+}
+
+declare void @init(i32*)
+


_______________________________________________
llvm-commits mailing list
llvm-commits at lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits



More information about the llvm-commits mailing list