rsmith added inline comments.
================
Comment at: lib/AST/Stmt.cpp:457-458
+ this->NumLabels = NumLabels;
+ assert((NumLabels != 0 && NumOutputs == 0) ||
+ (NumOutputs != 0 && NumLabels == 0));
----------------
This can be simplified a little:
```
assert(!(NumOutputs && NumLabels) && "asm goto cannot have outputs");
```
================
Comment at: lib/AST/Stmt.cpp:626
+ if (CheckLabels) {
+ if (N + 1 <= NumOperands - getNumLabels()) {
+ DiagOffs = CurPtr-StrStart-1;
----------------
`N < NumOperands - getNumLabels()`
================
Comment at: lib/AST/Stmt.cpp:628
+ DiagOffs = CurPtr-StrStart-1;
+ return diag::err_asm_invalid_operand_for_goto_labels;
+ }
----------------
I'm curious why we're checking this here, when all the other validation for the
modifier letter happens in LLVM. Does this check need to be done differently
from the others?
================
Comment at: lib/CodeGen/CGStmt.cpp:2186
+ }
+ StringRef Name = "normal";
+ Fallthrough = createBasicBlock(Name);
----------------
Nit: I think something like "asm.fallthrough" or "asm.cont" would be a more
obvious name for the fall-through block.
================
Comment at: lib/Sema/SemaStmtAsm.cpp:659-671
+ if (NS->getIdentifier(i))
+ MyList.emplace_back(std::make_pair(NS->getIdentifier(i)->getName(),
+ Exprs[i]));
+ for (unsigned i = NumOutputs, e = NumOutputs + NumInputs; i != e; ++i)
+ if (NS->getIdentifier(i))
+ MyList.emplace_back(std::make_pair(NS->getIdentifier(i)->getName(),
+ Exprs[i]));
----------------
This is still exposing / relying on an implementation detail of `GCCAsmStmt`.
Please replace `getIdentifier` with `getOuptutIdentifier` /
`getInputIdentifier` / `getLabelIdentifier`, adjust the indexing to match, and
remove `GCCAsmStmt::getIdentifier`. Or alternatively, iterate over `Names` here
rather than walking the parts of the `GCCAsmStmt`.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56571/new/
https://reviews.llvm.org/D56571
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
