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

[Sam Clegg via Phabricator via llvm-commits]

sbc100 added inline comments.


================
Comment at: lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp:716
+        auto &Op0 = Inst.getOperand(0);
+        if (Op0.getImm() == -1) Op0.setImm(Align);
+      }
----------------
aardappel wrote:
> 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.
On I didn't see that we have already calculated `IsLoadStore`.. can we just use that?


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63633/new/

https://reviews.llvm.org/D63633