This is an archive of the discontinued LLVM Phabricator instance.

[AVR] Fix incorrectly printed global symbol operands in inline assembly
ClosedPublic

Authored by benshi001 on Jan 19 2023, 2:12 AM.

Diff Detail

Event Timeline

benshi001 created this revision.Jan 19 2023, 2:12 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 19 2023, 2:12 AM
Herald added subscribers: Jim, hiraditya. · View Herald Transcript
benshi001 requested review of this revision.Jan 19 2023, 2:12 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 19 2023, 2:12 AM
benshi001 added inline comments.Jan 19 2023, 2:14 AM
llvm/test/CodeGen/AVR/inline-asm/inline-asm3.ll
76

Since no -mcpu option is specified in the test command, rcall is emitted as default instead of call. Because call is no valid for some early devices.

benshi001 edited the summary of this revision. (Show Details)Jan 19 2023, 2:15 AM
aykevl accepted this revision.Jan 19 2023, 6:22 AM

Looks good to me, with just a single comment. Feel free to fix this while committing.

llvm/lib/Target/AVR/AVRAsmPrinter.cpp
147

I think this should be the following, to be consistent with the rest of the code:

if (Error && MO.getType() == MachineOperand::MO_GlobalAddress) {

Even better would be to refactor the code to be more like RISC-V, to avoid the Error check (perhaps in a separate patch):

bool RISCVAsmPrinter::PrintAsmOperand(const MachineInstr *MI, unsigned OpNo,
                                      const char *ExtraCode, raw_ostream &OS) {
  // First try the generic code, which knows about modifiers like 'c' and 'n'.
  if (!AsmPrinter::PrintAsmOperand(MI, OpNo, ExtraCode, OS))
    return false;
This revision is now accepted and ready to land.Jan 19 2023, 6:22 AM
This revision was landed with ongoing or failed builds.Jan 19 2023, 5:45 PM
This revision was automatically updated to reflect the committed changes.

Looks good to me, with just a single comment. Feel free to fix this while committing.

Thanks. I will do the refactoring in another patch.

Looks good to me, with just a single comment. Feel free to fix this while committing.

I have made another patch for refactoring https://reviews.llvm.org/D142170