[PATCH] D63633: [WebAssembly] Fix p2align in assembler.
Wouter van Oortmerssen via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 24 11:38:15 PDT 2019
aardappel marked an inline comment as done.
aardappel added inline comments.
================
Comment at: lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp:716
+ auto &Op0 = Inst.getOperand(0);
+ if (Op0.getImm() == -1) Op0.setImm(Align);
+ }
----------------
sbc100 wrote:
> 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())
> }
> ```
Clang-formatted.
It is mildly-hacky, but I didn't want to duplicate the gigantic switch in `GetDefaultP2AlignAny`. Right now `GetDefaultP2AlignAny` is the source of truth for alignment in one place, which is also kinda nice.
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63633/new/
https://reviews.llvm.org/D63633
More information about the llvm-commits
mailing list