[PATCH] D140569: [AVR] Custom lower 32-bit shift instructions
Matt Arsenault via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sun Dec 25 07:45:25 PST 2022
arsenm added inline comments.
================
Comment at: llvm/lib/Target/AVR/AVRISelLowering.cpp:291
+ // 32-bit shifts are converted to a loop in IR.
+ llvm_unreachable("Expected a constant shift!");
+ }
----------------
aykevl wrote:
> benshi001 wrote:
> > `"Expected a constant shift amount!"`
> Fixed locally.
You can't guarantee this condition. If you don't want to handle this properly, should use emitError, or at least report_fatal_error and not unreachable. This could easily be run into by users
================
Comment at: llvm/lib/Target/AVR/AVRISelLowering.cpp:318
+ SDValue Result = DAG.getNode(Opc, dl, ResTys, SrcLo, SrcHi, Cnt);
+ return DAG.getNode(ISD::BUILD_PAIR, dl, MVT::i32, Result.getValue(0),
+ Result.getValue(1));
----------------
Why can't you handle everything here as a normal custom lowering? Why do you need the custom inserter?
================
Comment at: llvm/lib/Target/AVR/AVRISelLowering.cpp:1841
+ MachineRegisterInfo &MRI = BB->getParent()->getRegInfo();
+ DebugLoc dl = MI.getDebugLoc();
+
----------------
const ref
================
Comment at: llvm/lib/Target/AVR/AVRISelLowering.cpp:1908-1912
+ SmallVector<std::pair<Register, int>, 4> Registers;
+ Registers.push_back(std::pair(MI.getOperand(3).getReg(), AVR::sub_hi));
+ Registers.push_back(std::pair(MI.getOperand(3).getReg(), AVR::sub_lo));
+ Registers.push_back(std::pair(MI.getOperand(2).getReg(), AVR::sub_hi));
+ Registers.push_back(std::pair(MI.getOperand(2).getReg(), AVR::sub_lo));
----------------
Can directly initialize (also could use std::array?)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140569/new/
https://reviews.llvm.org/D140569
More information about the llvm-commits
mailing list