[PATCH] D63633: [WebAssembly] Fix p2align in assembler.

[Wouter van Oortmerssen via Phabricator via llvm-commits]

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