-
Notifications
You must be signed in to change notification settings - Fork 28
Fix various compile warnings #458
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: aie-public
Are you sure you want to change the base?
Conversation
// TODO Add Call Opcode e.g. JL | ||
default: | ||
break; | ||
} |
There was a problem hiding this comment.
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
llvm/lib/Target/AIE/AIEInstrInfo.cpp
Outdated
// const auto &TM = static_cast<const AIETargetMachine &>(MF.getTarget()); | ||
// return getInlineAsmLength(MI.getOperand(0).getSymbolName(), | ||
// *TM.getMCAsmInfo()); | ||
// } |
There was a problem hiding this comment.
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"; |
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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
I think you should update some copyright years. |
[AutoBump] Merge with fixes of b91ce7b (Oct 25) (2)
This PR fixes various compile warnings that affect builds treating warnings as errors.