[llvm] e9e2743 - [AVR][NFC] Improve some comment messages (#226423)
via llvm-commits
llvm-commits at lists.llvm.org
Sat Sep 26 04:31:51 PDT 2026
Author: Ben Shi
Date: 2026-09-26T13:31:45+02:00
New Revision: e9e27436e89127e5e42863cc6804945b3619b015
URL: https://github.com/llvm/llvm-project/commit/e9e27436e89127e5e42863cc6804945b3619b015
DIFF: https://github.com/llvm/llvm-project/commit/e9e27436e89127e5e42863cc6804945b3619b015.diff
LOG: [AVR][NFC] Improve some comment messages (#226423)
Added:
Modified:
llvm/lib/Target/AVR/AVRAsmPrinter.cpp
llvm/lib/Target/AVR/AVRDevices.td
llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp
llvm/lib/Target/AVR/AVRISelLowering.cpp
llvm/lib/Target/AVR/AVRInstrInfo.cpp
llvm/lib/Target/AVR/AVRInstrInfo.td
llvm/lib/Target/AVR/AVRRegisterInfo.cpp
llvm/lib/Target/AVR/MCTargetDesc/AVRMCAsmInfo.cpp
llvm/lib/Target/AVR/MCTargetDesc/AVRMCTargetDesc.h
Removed:
llvm/lib/Target/AVR/TODO.md
################################################################################
diff --git a/llvm/lib/Target/AVR/AVRAsmPrinter.cpp b/llvm/lib/Target/AVR/AVRAsmPrinter.cpp
index e7dd63fd39523..cd653dade917f 100644
--- a/llvm/lib/Target/AVR/AVRAsmPrinter.cpp
+++ b/llvm/lib/Target/AVR/AVRAsmPrinter.cpp
@@ -172,7 +172,7 @@ bool AVRAsmPrinter::PrintAsmMemoryOperand(const MachineInstr *MI,
assert(MO.isReg() && "Unexpected inline asm memory operand");
// TODO: We should be able to look up the alternative name for
- // the register if it's given.
+ // the register if it's given.
// TableGen doesn't expose a way of getting retrieving names
// for registers.
if (MI->getOperand(OpNum).getReg() == AVR::R31R30) {
diff --git a/llvm/lib/Target/AVR/AVRDevices.td b/llvm/lib/Target/AVR/AVRDevices.td
index ad760d7403573..71de1a7c6cf54 100644
--- a/llvm/lib/Target/AVR/AVRDevices.td
+++ b/llvm/lib/Target/AVR/AVRDevices.td
@@ -2,10 +2,10 @@
// AVR Device Definitions
//===---------------------------------------------------------------------===//
-// :TODO: Implement the skip errata, see `gcc/config/avr/avr-arch.h` for details
-// :TODO: We define all devices with SRAM to have all variants of LD/ST/LDD/STD.
-// In reality, avr1 (no SRAM) has one variant each of `LD` and `ST`.
-// avr2 (with SRAM) adds the rest of the variants.
+// TODO: Implement the skip errata, see `gcc/config/avr/avr-arch.h` for details
+// TODO: We define all devices with SRAM to have all variants of LD/ST/LDD/STD.
+// In reality, avr1 (no SRAM) has one variant each of `LD` and `ST`.
+// avr2 (with SRAM) adds the rest of the variants.
// A feature set aggregates features, grouping them. We don't want to create a
// new member in AVRSubtarget (to store a value) for each set because we do not
diff --git a/llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp b/llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp
index f9da7b3cfe7e9..f97e7a8ed08b2 100644
--- a/llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp
+++ b/llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp
@@ -1184,7 +1184,7 @@ bool AVRExpandPseudo::expand<AVR::STWPtrRr>(Block &MBB, BlockIt MBBI) {
bool SrcIsKill = MI.getOperand(1).isKill();
const AVRSubtarget &STI = MBB.getParent()->getSubtarget<AVRSubtarget>();
- //: TODO: need to reverse this order like inw and stsw?
+ // TODO: Need to reverse this order like inw and stsw?
if (STI.hasTinyEncoding()) {
// Handle this case in the expansion of STDWPtrQRr because it is very
@@ -2680,7 +2680,7 @@ bool AVRExpandPseudo::expandMI(Block &MBB, BlockIt MBBI) {
EXPAND(AVR::LDWRdPtr);
EXPAND(AVR::LDWRdPtrPi);
EXPAND(AVR::LDWRdPtrPd);
- case AVR::LDDWRdYQ: //: FIXME: remove this once PR13375 gets fixed
+ case AVR::LDDWRdYQ: // FIXME: Remove this once PR13375 gets fixed.
EXPAND(AVR::LDDWRdPtrQ);
EXPAND(AVR::LPMBRdZ);
EXPAND(AVR::LPMWRdZ);
diff --git a/llvm/lib/Target/AVR/AVRISelLowering.cpp b/llvm/lib/Target/AVR/AVRISelLowering.cpp
index 79ed56679120b..bb7caa275718a 100644
--- a/llvm/lib/Target/AVR/AVRISelLowering.cpp
+++ b/llvm/lib/Target/AVR/AVRISelLowering.cpp
@@ -195,9 +195,9 @@ AVRTargetLowering::AVRTargetLowering(const AVRTargetMachine &TM,
for (MVT VT : MVT::integer_valuetypes()) {
setOperationAction(ISD::SIGN_EXTEND_INREG, VT, Expand);
// TODO: The generated code is pretty poor. Investigate using the
- // same "shift and subtract with carry" trick that we do for
- // extending 8-bit to 16-bit. This may require infrastructure
- // improvements in how we treat 16-bit "registers" to be feasible.
+ // same "shift and subtract with carry" trick that we do for
+ // extending 8-bit to 16-bit. This may require infrastructure
+ // improvements in how we treat 16-bit "registers" to be feasible.
}
setMinFunctionAlignment(Align(2));
@@ -235,7 +235,7 @@ SDValue AVRTargetLowering::LowerShifts(SDValue Op, SelectionDAG &DAG) const {
if (ShiftAmount == 16) {
// Special case these two operations because they appear to be used by the
// generic codegen parts to lower 32-bit numbers.
- // TODO: perhaps we can lower shift amounts bigger than 16 to a 16-bit
+ // TODO: Perhaps we can lower shift amounts bigger than 16 to a 16-bit
// shift of a part of the 32-bit value?
switch (Op.getOpcode()) {
case ISD::SHL: {
@@ -2267,8 +2267,9 @@ AVRTargetLowering::insertWideShift(MachineInstr &MI,
// - lshr prefers starting from the least significant byte (1st case).
// - for ashr it depends on the number of shifted bytes.
// Some shift operations still don't get the most optimal mov sequences even
- // with this distinction. TODO: figure out why and try to fix it (but we're
- // already equal to or faster than avr-gcc in all cases except ashr 8).
+ // with this distinction.
+ // TODO: Figure out why and try to fix it (but we're
+ // already equal to or faster than avr-gcc in all cases except ashr 8).
if (Opc != ISD::SHL &&
(Opc != ISD::SRA || (ShiftAmt < 16 || ShiftAmt >= 22))) {
// Use the resulting registers starting with the least significant byte.
diff --git a/llvm/lib/Target/AVR/AVRInstrInfo.cpp b/llvm/lib/Target/AVR/AVRInstrInfo.cpp
index f58dbdae46dd9..15f176fafb121 100644
--- a/llvm/lib/Target/AVR/AVRInstrInfo.cpp
+++ b/llvm/lib/Target/AVR/AVRInstrInfo.cpp
@@ -90,7 +90,7 @@ Register AVRInstrInfo::isLoadFromStackSlot(const MachineInstr &MI,
int &FrameIndex) const {
switch (MI.getOpcode()) {
case AVR::LDDRdPtrQ:
- case AVR::LDDWRdYQ: { //: FIXME: remove this once PR13375 gets fixed
+ case AVR::LDDWRdYQ: { // FIXME: Remove this once PR13375 gets fixed.
if (MI.getOperand(1).isFI() && MI.getOperand(2).isImm() &&
MI.getOperand(2).getImm() == 0) {
FrameIndex = MI.getOperand(1).getIndex();
@@ -175,7 +175,7 @@ void AVRInstrInfo::loadRegFromStackSlot(MachineBasicBlock &MBB,
Opcode = AVR::LDDRdPtrQ;
} else if (TRI.isTypeLegalForClass(*RC, MVT::i16)) {
// Opcode = AVR::LDDWRdPtrQ;
- //: FIXME: remove this once PR13375 gets fixed
+ // FIXME: Remove this once PR13375 gets fixed.
Opcode = AVR::LDDWRdYQ;
} else {
llvm_unreachable("Cannot load this register from a stack slot!");
@@ -285,7 +285,7 @@ bool AVRInstrInfo::analyzeBranch(MachineBasicBlock &MBB,
}
// Handle unconditional branches.
- //: TODO: add here jmp
+ // TODO: Add here jmp.
if (I->getOpcode() == AVR::RJMPk) {
UnCondBrIter = I;
@@ -443,8 +443,8 @@ unsigned AVRInstrInfo::removeBranch(MachineBasicBlock &MBB,
if (I->isDebugInstr()) {
continue;
}
- //: TODO: add here the missing jmp instructions once they are implemented
- // like jmp, {e}ijmp, and other cond branches, ...
+ // TODO: Add here the missing jmp instructions once they are implemented
+ // like jmp, {e}ijmp, and other cond branches, ...
if (I->getOpcode() != AVR::RJMPk &&
getCondFromBranchOpc(I->getOpcode()) == AVRCC::COND_INVALID) {
break;
diff --git a/llvm/lib/Target/AVR/AVRInstrInfo.td b/llvm/lib/Target/AVR/AVRInstrInfo.td
index 4b5be413ba669..0f8a7621f56ea 100644
--- a/llvm/lib/Target/AVR/AVRInstrInfo.td
+++ b/llvm/lib/Target/AVR/AVRInstrInfo.td
@@ -692,8 +692,8 @@ let isCall = 1 in {
// SP is marked as a use to prevent stack-pointer assignments that appear
// immediately before calls from potentially appearing dead.
//
- // TODO: the imm field can be either 16 or 22 bits in devices with more
- // than 64k of ROM, fix it once we support the largest devices.
+ // TODO: The imm field can be either 16 or 22 bits in devices with more
+ // than 64k of ROM, fix it once we support the largest devices.
let Uses = [SP] in
def CALLk : F32BRk<0b111, (outs), (ins call_target:$k), "call\t$k",
[(AVRcall imm:$k)]>,
@@ -1407,8 +1407,8 @@ def SWAPRd : FRd<0b1001, 0b0100010, (outs GPR8:$rd), (ins GPR8:$src),
"swap\t$rd", [(set i8:$rd, (AVRSwap i8:$src))]>;
// IO register bit set/clear operations.
-//: TODO: add patterns when popcount(imm)==2 to be expanded with 2 sbi/cbi
-// instead of in+ori+out which requires one more instr.
+// TODO: Add patterns when popcount(imm)==2 to be expanded with 2 sbi/cbi
+// instead of in+ori+out which requires one more instr.
let hasSideEffects = 1, mayStore = 1 in {
def SBIAb : FIOBIT<0b10, (outs), (ins imm_port5:$addr, i8imm:$b),
"sbi\t$addr, $b",
@@ -1526,7 +1526,7 @@ def WDR : F16<0b1001010110101000, (outs), (ins), "wdr", []>;
// Pseudo instructions for later expansion
//===----------------------------------------------------------------------===//
-//: TODO: Optimize this for wider types AND optimize the following code
+// TODO: Optimize this for wider types AND optimize the following code
// compile int foo(char a, char b, char c, char d) {return d+b;}
// looks like a missed sext_inreg opportunity.
def SEXT : ExtensionPseudo<(outs DREGS:$dt), (ins GPR8:$src), "sext\t$dt, $src",
@@ -1656,8 +1656,9 @@ def CopyZero : Pseudo<(outs GPR8:$rd), (ins), "clrz\t$rd", [(set i8:$rd, 0)]>;
// Non-Instruction Patterns
//===----------------------------------------------------------------------===//
-//: TODO: look in x86InstrCompiler.td for odd encoding trick related to
-// add x, 128 -> sub x, -128. Clang is emitting an eor for this (ldi+eor)
+// TODO: Look in x86InstrCompiler.td for odd encoding trick related to
+// `add x, 128` -> `sub x, -128`. Clang is emitting an eor for this
+// (ldi+eor).
// the add instruction always writes the carry flag
def : Pat<(addc i8 : $src, i8 : $src2), (ADDRdRr i8 : $src, i8 : $src2)>;
@@ -1741,8 +1742,8 @@ def : Pat<(i16(AVRWrapper tblockaddress :$dst)), (LDIWRdK tblockaddress:$dst)>;
def : Pat<(i8(trunc(AVRlsrwn DLDREGS:$src, (i16 8)))),
(EXTRACT_SUBREG DREGS:$src, sub_hi)>;
-// :FIXME: DAGCombiner produces an shl node after legalization from these seq:
-// BR_JT -> (mul x, 2) -> (shl x, 1)
+// FIXME: DAGCombiner produces an shl node after legalization from these seq:
+// BR_JT -> (mul x, 2) -> (shl x, 1) .
def : Pat<(shl i16 : $src1, (i8 1)), (LSLWRd i16 : $src1)>;
// Lowering of 'tst' node to 'TST' instruction.
diff --git a/llvm/lib/Target/AVR/AVRRegisterInfo.cpp b/llvm/lib/Target/AVR/AVRRegisterInfo.cpp
index ac86ffb321a32..836faf8edfe19 100644
--- a/llvm/lib/Target/AVR/AVRRegisterInfo.cpp
+++ b/llvm/lib/Target/AVR/AVRRegisterInfo.cpp
@@ -209,8 +209,8 @@ bool AVRRegisterInfo::eliminateFrameIndex(MachineBasicBlock::iterator II,
// If the offset is too big we have to adjust and restore the frame pointer
// to materialize a valid load/store with displacement.
- //: TODO: consider using only one adiw/sbiw chain for more than one frame
- //: index
+ // TODO: Consider using only one adiw/sbiw chain for more than one frame
+ // indexes.
if (Offset > MaxOffset) {
unsigned AddOpc = AVR::ADIWRdK, SubOpc = AVR::SBIWRdK;
int AddOffset = Offset - MaxOffset;
diff --git a/llvm/lib/Target/AVR/MCTargetDesc/AVRMCAsmInfo.cpp b/llvm/lib/Target/AVR/MCTargetDesc/AVRMCAsmInfo.cpp
index b8f2d99442974..2d77e58145f22 100644
--- a/llvm/lib/Target/AVR/MCTargetDesc/AVRMCAsmInfo.cpp
+++ b/llvm/lib/Target/AVR/MCTargetDesc/AVRMCAsmInfo.cpp
@@ -200,7 +200,7 @@ bool AVRMCAsmInfo::evaluateAsRelocatableImpl(const MCSpecifierExpr &Expr,
if (E.getSpecifier() == AVR::S_PM)
Spec = AVR::S_PM;
- // TODO: don't attach specifier to MCSymbolRefExpr.
+ // TODO: Don't attach specifier to MCSymbolRefExpr.
Result =
MCValue::get(Value.getAddSym(), nullptr, Value.getConstant(), Spec);
}
diff --git a/llvm/lib/Target/AVR/MCTargetDesc/AVRMCTargetDesc.h b/llvm/lib/Target/AVR/MCTargetDesc/AVRMCTargetDesc.h
index e83d674f87cc9..e28074cbcd121 100644
--- a/llvm/lib/Target/AVR/MCTargetDesc/AVRMCTargetDesc.h
+++ b/llvm/lib/Target/AVR/MCTargetDesc/AVRMCTargetDesc.h
@@ -32,8 +32,7 @@ class Target;
MCInstrInfo *createAVRMCInstrInfo();
/// Creates a machine code emitter for AVR.
-MCCodeEmitter *createAVRMCCodeEmitter(const MCInstrInfo &MCII,
- MCContext &Ctx);
+MCCodeEmitter *createAVRMCCodeEmitter(const MCInstrInfo &MCII, MCContext &Ctx);
/// Creates an assembly backend for AVR.
MCAsmBackend *createAVRAsmBackend(const Target &T, const MCSubtargetInfo &STI,
diff --git a/llvm/lib/Target/AVR/TODO.md b/llvm/lib/Target/AVR/TODO.md
deleted file mode 100644
index 3a333355646d6..0000000000000
--- a/llvm/lib/Target/AVR/TODO.md
+++ /dev/null
@@ -1,7 +0,0 @@
-# Write an XFAIL test for this `FIXME` in `AVRInstrInfo.td`
-
-```
-// :FIXME: DAGCombiner produces an shl node after legalization from these seq:
-// BR_JT -> (mul x, 2) -> (shl x, 1)
-```
-
More information about the llvm-commits
mailing list