- Match the syntax output by InstPrinter.
- Fix it always emitting 0 for align. Had to work around fact that opcode is not available for GetDefaultP2Align while parsing.
- Updated tests that were erroneously happy with a p2align=0
Details
Diff Detail
- Repository
- rL LLVM
- Build Status
Buildable 33837 Build 33836: arc lint + arc unit
Event Timeline
Nice!
lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp | ||
---|---|---|
717 | Does llvm style allow this? clang-format? Is there some way we can tell if an opcode requires alignment? Calling GetDefaultP2AlignAny and checking for -1 seems bit hacky. Assuming I understand what happing I think this code would be more readable as: if (requiresAlignment() && !hasAlignment) { setImm(getDefaultAlign()) } |
lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp | ||
---|---|---|
717 | Clang-formatted. |
Thanks!
lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp | ||
---|---|---|
351–352 | Can atomic operations take p2align? Looks like alignments for atomic instructions are predefined. |
lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp | ||
---|---|---|
717 | On I didn't see that we have already calculated IsLoadStore.. can we just use that? |
Atomics only use default alignment.
lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp | ||
---|---|---|
351–352 | They are still stored in the encoding, and the tablegen def doesn't seem to make a difference between atomic and normal loads as to wether p2align can be changed, but yeah, the spec seems to indicate they are fixed. I can force atomics to use natural alignment for now. | |
717 | IsLoadStore is a bit of a hack that I would prefer to remove, rather than rely more on. I'd also have the get the instruction name in MatchAndEmitInstruction and repeat the same parsing there, so I think that will make the code generally worse. |
lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp | ||
---|---|---|
351–352 | Well, I presume they're in the encoding because we want to be able to relax the fixed alignment later.. if these instructions alignment is meant to be always fixed, they shouldn't be in the encoding. |
lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp | ||
---|---|---|
351–352 | What do you mean by encoding, you mean TableGen? I wrote that and I just copy-pasted that from normal memory instruction :$ |
lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp | ||
---|---|---|
351–352 | I mean the actual encoded bytes in the binary format. |
Can atomic operations take p2align? Looks like alignments for atomic instructions are predefined.