[HEXAGON] AddrModeOpt support for HVX and optimize adds #106368
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This patch does 3 things:
1. Add support for optimizing the address mode of HVX load/store
instructions
2. Reduce the value of Add instruction immediates by replacing with the
difference from other Addi instructions that share common base:
For Example, If we have the below sequence of instructions:
r1 = add(r2,llvm#1024)
...
r3 = add(r2,llvm#1152)
...
r4 = add(r2,llvm#1280)
Where the register r2 has the same reaching definition, They get modified to
the below sequence:
r1 = add(r2,llvm#1024)
...
r3 = add(r1,llvm#128)
...
r4 = add(r1,llvm#256)
3. Fixes a bug pass where the addi instructions were modified based on a
predicated register definition, leading to incorrect output.
Eg:
INST-1: if (p0) r2 = add(r13,llvm#128)
INST-2: r1 = add(r2,llvm#1024)
INST-3: r3 = add(r2,llvm#1152)
INST-4: r5 = add(r2,llvm#1280)
In the above case, since r2's definition is predicated, we do not want
to modify the uses of r2 in INST-3/INST-4 with add(r1,llvm#128/256)
4.Fixes a corner case
It looks like we never check whether the offset register is actually live (not clobbered)
at optimization site. Add the check whether it is live at MBB entrance.
The rest should have already been verified.
5. Fixes a bad codegen
For whatever reason we do transformation without checking if the value in register actually reaches the user.
This is second identical fix for this pass.
Co-authored-by: Anirudh Sundar <quic_sanirudh@quicinc.com>
Co-authored-by: Sergei Larin <slarin@quicinc.com>
|
@llvm/pr-subscribers-backend-hexagon Author: Abinaya Saravanan (quic-asaravan) ChangesThis patch does 3 things:
Eg: In the above case, since r2's definition is predicated, we do not want to modify the uses of r2 in INST-3/INST-4 with add(r1,#128/256) 4.Fixes a corner case It looks like we never check whether the offset register is actually live (not clobbered) at optimization site. Add the check whether it is live at MBB entrance. The rest should have already been verified.
For whatever reason we do transformation without checking if the value in register actually reaches the user. This is second identical fix for this pass. Co-authored-by: Anirudh Sundar <quic_sanirudh@quicinc.com> Patch is 30.69 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/106368.diff 5 Files Affected:
diff --git a/llvm/lib/Target/Hexagon/HexagonOptAddrMode.cpp b/llvm/lib/Target/Hexagon/HexagonOptAddrMode.cpp
index e7f5c257b21c1f..693649c2e8e581 100644
--- a/llvm/lib/Target/Hexagon/HexagonOptAddrMode.cpp
+++ b/llvm/lib/Target/Hexagon/HexagonOptAddrMode.cpp
@@ -35,6 +35,7 @@
#include "llvm/Support/Debug.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/raw_ostream.h"
+#include <algorithm>
#include <cassert>
#include <cstdint>
@@ -80,8 +81,10 @@ class HexagonOptAddrMode : public MachineFunctionPass {
private:
using MISetType = DenseSet<MachineInstr *>;
using InstrEvalMap = DenseMap<MachineInstr *, bool>;
+ DenseSet<MachineInstr *> ProcessedAddiInsts;
MachineRegisterInfo *MRI = nullptr;
+ const TargetRegisterInfo *TRI = nullptr;
const HexagonInstrInfo *HII = nullptr;
const HexagonRegisterInfo *HRI = nullptr;
MachineDominatorTree *MDT = nullptr;
@@ -93,6 +96,15 @@ class HexagonOptAddrMode : public MachineFunctionPass {
bool processBlock(NodeAddr<BlockNode *> BA);
bool xformUseMI(MachineInstr *TfrMI, MachineInstr *UseMI,
NodeAddr<UseNode *> UseN, unsigned UseMOnum);
+ bool processAddBases(NodeAddr<StmtNode *> AddSN, MachineInstr *AddMI);
+ bool usedInLoadStore(NodeAddr<StmtNode *> CurrentInstSN, int64_t NewOffset);
+ bool findFirstReachedInst(
+ MachineInstr *AddMI,
+ std::vector<std::pair<NodeAddr<StmtNode *>, NodeAddr<UseNode *>>>
+ &AddiList,
+ NodeAddr<StmtNode *> &UseSN);
+ bool updateAddBases(MachineInstr *CurrentMI, MachineInstr *FirstReachedMI,
+ int64_t NewOffset);
bool processAddUses(NodeAddr<StmtNode *> AddSN, MachineInstr *AddMI,
const NodeList &UNodeList);
bool updateAddUses(MachineInstr *AddMI, MachineInstr *UseMI);
@@ -207,8 +219,17 @@ bool HexagonOptAddrMode::canRemoveAddasl(NodeAddr<StmtNode *> AddAslSN,
return false;
for (auto &Mo : UseMI.operands())
+ // Is it a frame index?
if (Mo.isFI())
return false;
+ // Is the OffsetReg definition actually reaches UseMI?
+ if (!UseMI.getParent()->isLiveIn(OffsetReg) &&
+ MI.getParent() != UseMI.getParent()) {
+ LLVM_DEBUG(dbgs() << " The offset reg " << printReg(OffsetReg, TRI)
+ << " is NOT live in to MBB "
+ << UseMI.getParent()->getName() << "\n");
+ return false;
+ }
}
return true;
}
@@ -327,6 +348,14 @@ bool HexagonOptAddrMode::isSafeToExtLR(NodeAddr<StmtNode *> SN,
if ((LRExtRegDN.Addr->getFlags() & NodeAttrs::PhiRef) &&
MI->getParent() != UseMI->getParent())
return false;
+ // Is the OffsetReg definition actually reaches UseMI?
+ if (!UseMI->getParent()->isLiveIn(LRExtReg) &&
+ MI->getParent() != UseMI->getParent()) {
+ LLVM_DEBUG(dbgs() << " The LRExtReg reg " << printReg(LRExtReg, TRI)
+ << " is NOT live in to MBB "
+ << UseMI->getParent()->getName() << "\n");
+ return false;
+ }
}
return true;
}
@@ -344,6 +373,12 @@ bool HexagonOptAddrMode::isValidOffset(MachineInstr *MI, int Offset) {
case Hexagon::V6_vgathermhwq_pseudo:
return HII->isValidOffset(MI->getOpcode(), Offset, HRI, false);
default:
+ if (HII->getAddrMode(*MI) == HexagonII::BaseImmOffset) {
+ // The immediates are mentioned in multiples of vector counts
+ unsigned AlignMask = HII->getMemAccessSize(*MI) - 1;
+ if ((AlignMask & Offset) == 0)
+ return HII->isValidOffset(MI->getOpcode(), Offset, HRI, false);
+ }
return false;
}
}
@@ -414,6 +449,264 @@ unsigned HexagonOptAddrMode::getOffsetOpPosition(MachineInstr *MI) {
}
}
+bool HexagonOptAddrMode::usedInLoadStore(NodeAddr<StmtNode *> CurrentInstSN,
+ int64_t NewOffset) {
+ NodeList LoadStoreUseList;
+
+ getAllRealUses(CurrentInstSN, LoadStoreUseList);
+ bool FoundLoadStoreUse = false;
+ for (auto I = LoadStoreUseList.begin(), E = LoadStoreUseList.end(); I != E;
+ ++I) {
+ NodeAddr<UseNode *> UN = *I;
+ NodeAddr<StmtNode *> SN = UN.Addr->getOwner(*DFG);
+ MachineInstr *LoadStoreMI = SN.Addr->getCode();
+ const MCInstrDesc &MID = LoadStoreMI->getDesc();
+ if ((MID.mayLoad() || MID.mayStore()) &&
+ isValidOffset(LoadStoreMI, NewOffset)) {
+ FoundLoadStoreUse = true;
+ break;
+ }
+ }
+ return FoundLoadStoreUse;
+}
+
+bool HexagonOptAddrMode::findFirstReachedInst(
+ MachineInstr *AddMI,
+ std::vector<std::pair<NodeAddr<StmtNode *>, NodeAddr<UseNode *>>> &AddiList,
+ NodeAddr<StmtNode *> &UseSN) {
+ // Find the very first Addi instruction in the current basic block among the
+ // AddiList This is the Addi that should be preserved so that we do not need
+ // to handle the complexity of moving instructions
+ //
+ // TODO: find Addi instructions across basic blocks
+ //
+ // TODO: Try to remove this and add a solution that optimizes the number of
+ // Addi instructions that can be modified.
+ // This change requires choosing the Addi with the median offset value, but
+ // would also require moving that instruction above the others. Since this
+ // pass runs after register allocation, there might be multiple cases that
+ // need to be handled if we move instructions around
+ MachineBasicBlock *CurrentMBB = AddMI->getParent();
+ for (auto &InstIter : *CurrentMBB) {
+ // If the instruction is an Addi and is in the AddiList
+ if (InstIter.getOpcode() == Hexagon::A2_addi) {
+ auto Iter = std::find_if(
+ AddiList.begin(), AddiList.end(), [&InstIter](const auto &SUPair) {
+ return SUPair.first.Addr->getCode() == &InstIter;
+ });
+ if (Iter != AddiList.end()) {
+ UseSN = Iter->first;
+ return true;
+ }
+ }
+ }
+ return false;
+}
+
+// This function tries to modify the immediate value in Hexagon::Addi
+// instructions, so that the immediates could then be moved into a load/store
+// instruction with offset and the add removed completely when we call
+// processAddUses
+//
+// For Example, If we have the below sequence of instructions:
+//
+// r1 = add(r2,#1024)
+// ...
+// r3 = add(r2,#1152)
+// ...
+// r4 = add(r2,#1280)
+//
+// Where the register r2 has the same reaching definition, They get modified to
+// the below sequence:
+//
+// r1 = add(r2,#1024)
+// ...
+// r3 = add(r1,#128)
+// ...
+// r4 = add(r1,#256)
+//
+// The below change helps the processAddUses method to later move the
+// immediates #128 and #256 into a load/store instruction that can take an
+// offset, like the Vd = mem(Rt+#s4)
+bool HexagonOptAddrMode::processAddBases(NodeAddr<StmtNode *> AddSN,
+ MachineInstr *AddMI) {
+
+ bool Changed = false;
+
+ LLVM_DEBUG(dbgs() << "\n\t\t[Processing Addi]: " << *AddMI << "\n");
+
+ auto Processed =
+ [](const MachineInstr *MI,
+ const DenseSet<MachineInstr *> &ProcessedAddiInsts) -> bool {
+ // If we've already processed this Addi, just return
+ if (ProcessedAddiInsts.find(MI) != ProcessedAddiInsts.end()) {
+ LLVM_DEBUG(dbgs() << "\t\t\tAddi already found in ProcessedAddiInsts: "
+ << *MI << "\n\t\t\tSkipping...");
+ return true;
+ }
+ return false;
+ };
+
+ if (Processed(AddMI, ProcessedAddiInsts))
+ return Changed;
+ ProcessedAddiInsts.insert(AddMI);
+
+ // Get the base register that would be shared by other Addi Intructions
+ Register BaseReg = AddMI->getOperand(1).getReg();
+
+ // Store a list of all Addi instructions that share the above common base
+ // register
+ std::vector<std::pair<NodeAddr<StmtNode *>, NodeAddr<UseNode *>>> AddiList;
+
+ NodeId UAReachingDefID;
+ // Find the UseNode that contains the base register and it's reachingDef
+ for (NodeAddr<UseNode *> UA : AddSN.Addr->members_if(DFG->IsUse, *DFG)) {
+ RegisterRef URR = UA.Addr->getRegRef(*DFG);
+ if (BaseReg != URR.Reg)
+ continue;
+
+ UAReachingDefID = UA.Addr->getReachingDef();
+ NodeAddr<DefNode *> UADef = DFG->addr<DefNode *>(UAReachingDefID);
+ if (!UAReachingDefID || UADef.Addr->getFlags() & NodeAttrs::PhiRef) {
+ LLVM_DEBUG(dbgs() << "\t\t\t Could not find reachingDef. Skipping...\n");
+ return false;
+ }
+ }
+
+ NodeAddr<DefNode *> UAReachingDef = DFG->addr<DefNode *>(UAReachingDefID);
+ NodeAddr<StmtNode *> ReachingDefStmt = UAReachingDef.Addr->getOwner(*DFG);
+
+ // If the reaching definition is a predicated instruction, this might not be
+ // the only definition of our base register, so return immediately.
+ MachineInstr *ReachingDefInstr = ReachingDefStmt.Addr->getCode();
+ if (HII->isPredicated(*ReachingDefInstr))
+ return false;
+
+ NodeList AddiUseList;
+
+ // Find all Addi instructions that share the same base register and add them
+ // to the AddiList
+ getAllRealUses(ReachingDefStmt, AddiUseList);
+ for (auto I = AddiUseList.begin(), E = AddiUseList.end(); I != E; ++I) {
+ NodeAddr<UseNode *> UN = *I;
+ NodeAddr<StmtNode *> SN = UN.Addr->getOwner(*DFG);
+ MachineInstr *MI = SN.Addr->getCode();
+
+ // Only add instructions if it's an Addi and it's not already processed.
+ if (MI->getOpcode() == Hexagon::A2_addi &&
+ !(MI != AddMI && Processed(MI, ProcessedAddiInsts))) {
+ AddiList.push_back({SN, UN});
+
+ // This ensures that we process each instruction only once
+ ProcessedAddiInsts.insert(MI);
+ }
+ }
+
+ // If there's only one Addi instruction, nothing to do here
+ if (AddiList.size() <= 1)
+ return Changed;
+
+ NodeAddr<StmtNode *> FirstReachedUseSN;
+ // Find the first reached use of Addi instruction from the list
+ if (!findFirstReachedInst(AddMI, AddiList, FirstReachedUseSN))
+ return Changed;
+
+ // If we reach this point we know that the StmtNode FirstReachedUseSN is for
+ // an Addi instruction. So, we're guaranteed to have just one DefNode, and
+ // hence we can access the front() directly without checks
+ NodeAddr<DefNode *> FirstReachedUseDN =
+ FirstReachedUseSN.Addr->members_if(DFG->IsDef, *DFG).front();
+
+ MachineInstr *FirstReachedMI = FirstReachedUseSN.Addr->getCode();
+ const MachineOperand FirstReachedMIImmOp = FirstReachedMI->getOperand(2);
+ if (!FirstReachedMIImmOp.isImm())
+ return false;
+
+ for (auto &I : AddiList) {
+ NodeAddr<StmtNode *> CurrentInstSN = I.first;
+ NodeAddr<UseNode *> CurrentInstUN = I.second;
+
+ MachineInstr *CurrentMI = CurrentInstSN.Addr->getCode();
+ MachineOperand &CurrentMIImmOp = CurrentMI->getOperand(2);
+
+ int64_t NewOffset;
+
+ // Even though we know it's an Addi instruction, the second operand could be
+ // a global value and not an immediate
+ if (!CurrentMIImmOp.isImm())
+ continue;
+
+ NewOffset = CurrentMIImmOp.getImm() - FirstReachedMIImmOp.getImm();
+
+ // This is the first occuring Addi, so skip modifying this
+ if (CurrentMI == FirstReachedMI) {
+ continue;
+ }
+
+ if (CurrentMI->getParent() != FirstReachedMI->getParent())
+ continue;
+
+ // Modify the Addi instruction only if it could be used to modify a
+ // future load/store instruction and get removed
+ //
+ // This check is needed because, if we modify the current Addi instruction
+ // we create RAW dependence between the FirstReached Addi and the current
+ // one, which could result in extra packets. So we only do this change if
+ // we know the current Addi would get removed later
+ if (!usedInLoadStore(CurrentInstSN, NewOffset)) {
+ return false;
+ }
+
+ // Verify whether the First Addi's definition register is still live when
+ // we reach the current Addi
+ RegisterRef FirstReachedDefRR = FirstReachedUseDN.Addr->getRegRef(*DFG);
+ NodeAddr<InstrNode *> CurrentAddiIN = CurrentInstUN.Addr->getOwner(*DFG);
+ NodeAddr<RefNode *> NearestAA =
+ LV->getNearestAliasedRef(FirstReachedDefRR, CurrentAddiIN);
+ if ((DFG->IsDef(NearestAA) && NearestAA.Id != FirstReachedUseDN.Id) ||
+ (!DFG->IsDef(NearestAA) &&
+ NearestAA.Addr->getReachingDef() != FirstReachedUseDN.Id)) {
+ // Found another definition of FirstReachedDef
+ LLVM_DEBUG(dbgs() << "\t\t\tCould not modify below Addi since the first "
+ "defined Addi register was redefined\n");
+ continue;
+ }
+
+ MachineOperand CurrentMIBaseOp = CurrentMI->getOperand(1);
+ if (CurrentMIBaseOp.getReg() != FirstReachedMI->getOperand(1).getReg()) {
+ continue;
+ }
+
+ // If we reached this point, then we can modify MI to use the result of
+ // FirstReachedMI
+ Changed |= updateAddBases(CurrentMI, FirstReachedMI, NewOffset);
+
+ // Update the reachingDef of the Current AddI use after change
+ CurrentInstUN.Addr->linkToDef(CurrentInstUN.Id, FirstReachedUseDN);
+ }
+
+ return Changed;
+}
+
+bool HexagonOptAddrMode::updateAddBases(MachineInstr *CurrentMI,
+ MachineInstr *FirstReachedMI,
+ int64_t NewOffset) {
+ LLVM_DEBUG(dbgs() << "[About to modify the Addi]: " << *CurrentMI << "\n");
+ const MachineOperand FirstReachedDef = FirstReachedMI->getOperand(0);
+ Register FirstDefRegister = FirstReachedDef.getReg();
+
+ MachineOperand &CurrentMIBaseOp = CurrentMI->getOperand(1);
+ MachineOperand &CurrentMIImmOp = CurrentMI->getOperand(2);
+
+ CurrentMIBaseOp.setReg(FirstDefRegister);
+ CurrentMIBaseOp.setIsUndef(FirstReachedDef.isUndef());
+ CurrentMIBaseOp.setImplicit(FirstReachedDef.isImplicit());
+ CurrentMIImmOp.setImm(NewOffset);
+ ProcessedAddiInsts.insert(CurrentMI);
+ MRI->clearKillFlags(FirstDefRegister);
+ return true;
+}
+
bool HexagonOptAddrMode::processAddUses(NodeAddr<StmtNode *> AddSN,
MachineInstr *AddMI,
const NodeList &UNodeList) {
@@ -737,7 +1030,6 @@ bool HexagonOptAddrMode::changeAddAsl(NodeAddr<UseNode *> AddAslUN,
for (unsigned i = OpStart; i < OpEnd; ++i)
MIB.add(UseMI->getOperand(i));
-
Deleted.insert(UseMI);
}
@@ -782,6 +1074,8 @@ bool HexagonOptAddrMode::processBlock(NodeAddr<BlockNode *> BA) {
<< "]: " << *MI << "\n\t[InstrNode]: "
<< Print<NodeAddr<InstrNode *>>(IA, *DFG) << '\n');
+ if (MI->getOpcode() == Hexagon::A2_addi)
+ Changed |= processAddBases(SA, MI);
NodeList UNodeList;
getAllRealUses(SA, UNodeList);
@@ -869,6 +1163,7 @@ bool HexagonOptAddrMode::runOnMachineFunction(MachineFunction &MF) {
bool Changed = false;
auto &HST = MF.getSubtarget<HexagonSubtarget>();
MRI = &MF.getRegInfo();
+ TRI = MF.getSubtarget().getRegisterInfo();
HII = HST.getInstrInfo();
HRI = HST.getRegisterInfo();
const auto &MDF = getAnalysis<MachineDominanceFrontier>();
@@ -885,6 +1180,7 @@ bool HexagonOptAddrMode::runOnMachineFunction(MachineFunction &MF) {
LV = &L;
Deleted.clear();
+ ProcessedAddiInsts.clear();
NodeAddr<FuncNode *> FA = DFG->getFunc();
LLVM_DEBUG(dbgs() << "==== [RefMap#]=====:\n "
<< Print<NodeAddr<FuncNode *>>(FA, *DFG) << "\n");
diff --git a/llvm/test/CodeGen/Hexagon/addrmode-opt-nonreaching.mir b/llvm/test/CodeGen/Hexagon/addrmode-opt-nonreaching.mir
new file mode 100644
index 00000000000000..b5ea68f9961c18
--- /dev/null
+++ b/llvm/test/CodeGen/Hexagon/addrmode-opt-nonreaching.mir
@@ -0,0 +1,232 @@
+# It is not safe to make the transformation if the definition is killed by a call.
+# Use debug output for simplicity and test resilience.
+#
+# RUN: llc -march=hexagon -run-pass amode-opt %s -debug -o %t_1.mir 2>&1 | FileCheck %s
+#CHECK: The LRExtReg reg {{.*}} is NOT live in to MBB
+--- |
+ ; ModuleID = 'foo.reduced.i'
+ source_filename = "foo.reduced.i"
+ target datalayout = "e-m:e-p:32:32:32-a:0-n16:32-i64:64:64-i32:32:32-i16:16:16-i1:8:8-f32:32:32-f64:64:64-v32:32:32-v64:64:64-v512:512:512-v1024:1024:1024-v2048:2048:2048"
+ target triple = "hexagon-unknown-unknown-elf"
+
+ %struct.struct_1 = type { i8, i8, [7 x %struct.struct_3] }
+ %struct.struct_3 = type { i8, %struct.struct_4 }
+ %struct.struct_4 = type { i32, i32 }
+ %struct.struct_2 = type { i32, i32 }
+
+ ; Function Attrs: nounwind
+ define dso_local zeroext i8 @fun_4(i32 noundef %arg_1, i8 noundef zeroext %arg_2, ptr nocapture noundef readonly %arg_3, ptr nocapture noundef readonly %arg_4) local_unnamed_addr #0 {
+ entry:
+ %conv = zext i8 %arg_2 to i32
+ %cmp = icmp ult i8 %arg_2, 7
+ br i1 %cmp, label %if.then, label %if.end
+
+ if.then: ; preds = %entry
+ tail call void @fun_2(ptr noundef null, i32 noundef %conv) #3
+ unreachable
+
+ if.end: ; preds = %entry
+ %cgep = getelementptr inbounds %struct.struct_1, ptr %arg_3, i32 0, i32 2, i32 %conv
+ %cgep23 = getelementptr inbounds %struct.struct_3, ptr %cgep, i32 0, i32 1, i32 1
+ %0 = load i32, ptr %cgep23, align 4
+ %cmp3 = icmp eq i32 %0, 4
+ br i1 %cmp3, label %land.lhs.true, label %if.else
+
+ land.lhs.true: ; preds = %if.end
+ %cgep22 = getelementptr inbounds %struct.struct_3, ptr %cgep, i32 0, i32 1
+ %1 = load i32, ptr %cgep22, align 4
+ %call = tail call zeroext i8 @fun_5(i32 noundef %arg_1, i32 noundef %1) #4
+ %tobool.not = icmp eq i8 %call, 0
+ br i1 %tobool.not, label %if.else, label %if.end12
+
+ if.else: ; preds = %land.lhs.true, %if.end
+ %2 = load i8, ptr %cgep, align 4
+ %cmp.i = icmp eq i8 %2, 1
+ br i1 %cmp.i, label %if.then.i, label %fun_3.exit
+
+ if.then.i: ; preds = %if.else
+ %cgep2027 = bitcast ptr %arg_4 to ptr
+ %cgep26 = getelementptr inbounds %struct.struct_2, ptr %cgep2027, i32 0, i32 1
+ %3 = load i32, ptr %cgep26, align 4
+ %call.i = tail call i32 @fun_1(i32 noundef %arg_1) #4
+ %cmp2.i = icmp ult i32 %3, %call.i
+ %cgep25 = getelementptr inbounds %struct.struct_2, ptr %cgep2027, i32 0, i32 1
+ %4 = load i32, ptr %cgep25, align 4
+ %call6.i = tail call i32 @fun_1(i32 noundef %arg_1) #4
+ %cmp7.i = icmp ult i32 %4, %call6.i
+ %5 = select i1 %cmp2.i, i1 true, i1 %cmp7.i
+ br label %fun_3.exit
+
+ fun_3.exit: ; preds = %if.else, %if.then.i
+ %resume_ttl.1.i = phi i1 [ false, %if.else ], [ %5, %if.then.i ]
+ %conv15.i = zext i1 %resume_ttl.1.i to i8
+ br label %if.end12
+
+ if.end12: ; preds = %land.lhs.true, %fun_3.exit
+ %resume_loops.0 = phi i8 [ %conv15.i, %fun_3.exit ], [ 1, %land.lhs.true ]
+ ret i8 %resume_loops.0
+ }
+
+ ; Function Attrs: noreturn
+ declare dso_local void @fun_2(ptr noundef, i32 noundef) local_unnamed_addr #1
+
+ declare dso_local zeroext i8 @fun_5(i32 noundef, i32 noundef) local_unnamed_addr #2
+
+ declare dso_local i32 @fun_1(i32 noundef) local_unnamed_addr #2
+
+ !llvm.module.flags = !{!0, !1}
+ !llvm.ident = !{!2}
+
+ !0 = !{i32 1, !"wchar_size", i32 4}
+ !1 = !{i32 7, !"frame-pointer", i32 2}
+ !2 = !{!"QuIC LLVM Hexagon Clang version 8.9 Engineering Release: hexagon-clang-89"}
+
+...
+---
+name: fun_4
+alignment: 16
+exposesReturnsTwice: false
+legalized: false
+regBankSelected: false
+selected: false
+failedISel: false
+tracksRegLiveness: true
+hasWinCFI: false
+callsEHReturn: false
+callsUnwindInit: false
+hasEHCatchret: false
+hasEHScopes: false
+hasEHFunclets: false
+isOutlined: false
+debugInstrRef: false
+failsVerification: false
+tracksDebugUserValues: true
+registers: []
+liveins:
+ - { reg: '$r0', virtual-reg: '' }
+ - { reg: '$r1', virtual-reg: '' }
+ - { reg: '$r2', virtual-reg: '' }
+ - { reg: '$r3', virtual-reg: '' }
+frameInfo:
+ isFrameAddressTaken: false
+ isReturnAddressTaken: false
+ hasStackMap: false
+ hasPatchPoint: false
+ stackSize: 0
+ offsetAdjustment: 0
+ maxAlignment: 4
+ adjustsStack: false
+ hasCalls: true
+ stackProtector: ''
+ functionContext: ''
+ maxCallFrameSize: 4294967295
+ cvBytesOfCalleeSavedRegiste...
[truncated]
|
|
@SundeepKushwaha @iajbar @quic-sanirudh @JyotsnaVerma Can you pls review this? |
|
This patch fixes this issue #80185 |
androm3da
reviewed
Aug 29, 2024
quic-asaravan
changed the title
|
LGTM but is it possible @nathanchance to confirm it addresses the compile-time issue? |
androm3da
approved these changes
Sep 13, 2024
I’ll double check and get back to you by the end of the day. |
|
This appears to resolve the compile time impact for me. Just running |
iajbar
approved these changes
Sep 13, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch does 3 things:
Add support for optimizing the address mode of HVX load/store instructions
Reduce the value of Add instruction immediates by replacing with the difference from other Addi instructions that share common base:
For Example, If we have the below sequence of instructions: r1 = add(r2,# 1024) ... r3 = add(r2,# 1152) ... r4 = add(r2,# 1280)
Where the register r2 has the same reaching definition, They get modified to the below sequence:
Fixes a bug pass where the addi instructions were modified based on a predicated register definition, leading to incorrect output.
Eg:
INST-1: if (p0) r2 = add(r13,# 128)
INST-2: r1 = add(r2,# 1024)
INST-3: r3 = add(r2,# 1152)
INST-4: r5 = add(r2,# 1280)
In the above case, since r2's definition is predicated, we do not want to modify the uses of r2 in INST-3/INST-4 with add(r1,#128/256)
4.Fixes a corner case
It looks like we never check whether the offset register is actually live (not clobbered) at optimization site. Add the check whether it is live at MBB entrance. The rest should have already been verified.
For whatever reason we do transformation without checking if the value in register actually reaches the user. This is second identical fix for this pass.
Co-authored-by: Anirudh Sundar quic_sanirudh@quicinc.com
Co-authored-by: Sergei Larin slarin@quicinc.com