rSerge added inline comments.
================
Comment at: lib/Driver/Tools.cpp:4903-4906
if (Triple.getOS() == llvm::Triple::Linux &&
(Triple.getArch() == llvm::Triple::arm ||
- Triple.getArch() == llvm::Triple::x86_64)) {
+ Triple.getArch() == llvm::Triple::x86_64 ||
+ Triple.getArch() == llvm::Triple::aarch64)) {
----------------
dberris wrote:
> rSerge wrote:
> > dberris wrote:
> > > I'm wondering whether it's worth turning this into a `switch` statement
> > > now that we have more than two supported architectures?
> > I think that would lead to more awkward code: there wouldn't be a single
> > decision outcome point (like the current `else` block), so to avoid
> > duplicating the code which currently prints the message, a `bool` variable
> > would be needed. I think it's more neat to just enumerate all the OS&CPU
> > combinations in the `if` condition for now.
> > This place is not performance-critical and the compiler should convert
> > appropriate `if`s into `switch`es anyway.
> This is an issue of making it more readable, something like:
>
> ```
> if (Triple.getOS() != llvm::Triple::Linux)
> D.Diag(...) << ...; // Unsupported OS.
> switch (Triple.getArch()) {
> case llvm::Triple::arm:
> case llvm::Triple::x86_64:
> case llvm::Tripe::aarch64:
> // Supported.
> break;
> default:
> D.Diag(...) << ...;
> }
> ```
>
> This way any new architectures become supported, they just get added to the
> list of cases that short-circuit.
We can't say that an OS is supported or unsupported unless all CPU
architectures for this OS support or don't support XRay, and this is not going
to happen in the near future. So it is more accurate to say about the triple:
some triples are supported and some are not. So in coding it is natural to
check for the triple with `||` and `&&`.
https://reviews.llvm.org/D26415
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
