[PATCH] D88451: [ms] [llvm-ml] Create the @feat.00 symbol, and populate it appropriately

[Eric Astor via Phabricator via llvm-commits]

epastor marked 2 inline comments as done.
epastor added inline comments.


================
Comment at: llvm/tools/llvm-ml/llvm-ml.cpp:372
+    // link.exe.
+    int64_t Feat00Flags = 0b10;
+    if (SafeSEH) {
----------------
rnk wrote:
> This looks like the first instance of C++14 binary literals in LLVM non-test C++ source code. In theory, C++14 support is required, but if I were you I'd prefer not to be the guinea pig.
Switched, thanks.


================
Comment at: llvm/tools/llvm-ml/llvm-ml.cpp:380-381
+    }
+    MCSymbol *Feat00Sym = Ctx.getOrCreateSymbol("@feat.00");
+    if (Feat00Sym) {
+      Feat00Sym->setRedefinable(true);
----------------
rnk wrote:
> Can getOrCreateSymbol ever return null? The implementation suggests that it can't. Is this branch meant to avoid redefining @feat.00 if it is already set?
I think I was assuming it could be null when I wrote this. Removed the unnecessary branch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88451