Skip to content

Conversation

philippjh
Copy link
Member

@philippjh philippjh commented May 7, 2025

This PR fixes various compile warnings that affect builds treating warnings as errors.

// TODO Add Call Opcode e.g. JL
default:
break;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this removes a TODO

// const auto &TM = static_cast<const AIETargetMachine &>(MF.getTarget());
// return getInlineAsmLength(MI.getOperand(0).getSymbolName(),
// *TM.getMCAsmInfo());
// }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove the commented code?

o << " return nullptr;\n";
for (unsigned int i = 0; i < PseudoInstFormats.size(); i++)
PseudoInstFormats[i].emitAlternateInstsOpcode(o, i);
o << " }\n}\n";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This complicates the generator, while the code is correct. Can we disable these warnings in generated code?

@@ -40,7 +40,8 @@ class AIE final : public TargetInfo {
uint32_t calcEFlags() const override;
RelExpr getRelExpr(RelType Type, const Symbol &S,
const uint8_t *Loc) const override;
void relocate(uint8_t *Loc, const Relocation &rel, uint64_t Val) const override;
void relocate(uint8_t *Loc, const Relocation &rel,
uint64_t Val) const override;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about lld coding style, but I would say that rel should be Rel.

//73 : (symbol_addr_AR + addend ) : addr [19..0]@0 in w32[1] // with default addend 0
// 72 : (symbol_addr_AR + addend ) : addr [19..0]@0 in w08[4] //
// with default addend 0 73 : (symbol_addr_AR + addend ) : addr [19..0]@0
// in w32[1] // with default addend 0
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this requires a manual wrap

@martien-de-jong
Copy link
Collaborator

I think you should update some copyright years.

mgehre-amd pushed a commit that referenced this pull request Aug 21, 2025
[AutoBump] Merge with fixes of b91ce7b (Oct 25) (2)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants