[PATCH] D18242: [TableGen] AsmMatcher: support for default values for optional operands

[Ahmed Bougacha via llvm-commits]

ab added a comment.

I'm no fan of this, but I think you're right, there's not much better we can do right now.

What do you think of only generating the extra code only if there's at least one optional operand class?  That would avoid needing a mask on other targets.


================
Comment at: include/llvm/Target/Target.td:623
@@ +622,3 @@
+
+  /// The name of the method on the target specific asm parser that return
+  /// default operand for this optional operand. This method have impact only
----------------
ab wrote:
> "This method is only used if.."
return default -> returns the default

================
Comment at: include/llvm/Target/Target.td:623-624
@@ +622,4 @@
+
+  /// The name of the method on the target specific asm parser that return
+  /// default operand for this optional operand. This method have impact only
+  /// if IsOptional == 0. If not set, this will default to
----------------
"This method is only used if.."

================
Comment at: include/llvm/Target/Target.td:625
@@ +624,3 @@
+  /// default operand for this optional operand. This method have impact only
+  /// if IsOptional == 0. If not set, this will default to
+  /// "defaultFooOperands", where Foo is the AsmOperandClass name. The method
----------------
IsOptional == 1, no?

================
Comment at: include/llvm/Target/Target.td:628-630
@@ +627,5 @@
+  /// signature should be:
+  ///   std::unique_ptr<MCParsedAsmOperand> defaultFooOperands(
+  ///           unsigned Opcode,
+  ///           const OperandVector &Operands) const;
+  string DefaultMethod = ?;
----------------
Why pass anything?  You don't use the Opcode, and the Operands are only used for the location:
- what happens when the first and only operand is optional?
- loc of the preceding operand is probably more accurate; but I think just using SMLoc() makes the most sense
- does anything look at locations at this point anyway? If there's an error it would have been caught earlier, no?

================
Comment at: utils/TableGen/AsmMatcherEmitter.cpp:207
@@ -206,1 +206,3 @@
 
+  /// DefaultMethod - The name of the method
+  std::string DefaultMethod;
----------------
which method?


http://reviews.llvm.org/D18242