[PATCH] D48935: [llvm-exegesis] Provide a way to handle memory instructions.

Clement Courbet via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 19 06:53:48 PDT 2018


courbet added inline comments.


================
Comment at: tools/llvm-exegesis/lib/Uops.cpp:182
+    Prototype.Snippet.push_back(std::move(II));
+    if (HasMemOperands)
+      instantiateMemoryOperands(Prototype.ScratchSpaceReg, Prototype.Snippet);
----------------
gchatelet wrote:
> courbet wrote:
> > gchatelet wrote:
> > > This is not ideal because `if (HasMemOperands) instantiateMemoryOperands(...)` is repeated before each return.
> > > How about a `PrototypeBuilder` struct that encapsulate the memory logic ?
> > I originally had a lambda called `MaybeInstanciateMemoryOperands`, but TBH I think the repeated `if` makes it clearer.
> > I originally had a lambda called MaybeInstanciateMemoryOperands, but TBH I think the repeated if makes it clearer.
> 
> The `if` statement might be clearer than the Lambda but it is as error prone (you can omit the lambda and it compiles but it's broken).
> My point was, if you use a Builder it is correct by design (and it removes visual clutter), true that it adds a struct to the file though.
Here's a proposal with no lambda and no builder, tell me what you think.


Repository:
  rL LLVM

https://reviews.llvm.org/D48935





More information about the llvm-commits mailing list