[PATCH] D34878: [ARM] Option for reading thread pointer from coprocessor register

[Kristof Beyls via Phabricator via cfe-commits]

kristof.beyls added inline comments.


================
Comment at: lib/Driver/ToolChains/Arch/ARM.cpp:145-146
+  // choose soft mode.
+  if (ThreadPointer == ReadTPMode::Invalid)
+    ThreadPointer = ReadTPMode::Soft;
+  return ThreadPointer;
----------------
spetrovic wrote:
> kristof.beyls wrote:
> > .... and always give an error if an invalid mtp command line option was given, rather than default back to soft mode?
> If 'mtp' takes invalid value, error is always provided. This is the case when there is no -mtp option in command line, you can see how the case of invalid mtp argument is handled in the code above.
Right.
I just thought that the function would be ever so slightly simpler if it had the following structure roughly:

```
if (Arg *A = ...) {
  ThreadPointer = llvm::StringSwitch... ;
  if (!Invalid)
    return ThreadPointer;
  if (empty)
    D.Diag();
  else
    D.Diag();
  return Invalid;
}
return ReadTPMode::Soft;
```

And probably is also slightly closer to the coding standard described in https://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code
But this is a really minor comment.




https://reviews.llvm.org/D34878