jhuber6 added inline comments.

================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3209-3214
+      else if ((T.isArch64Bit() && TT.isArch32Bit()) ||
+               (T.isArch64Bit() && TT.isArch16Bit()) ||
+               (T.isArch32Bit() && TT.isArch64Bit()) ||
+               (T.isArch32Bit() && TT.isArch16Bit()) ||
+               (T.isArch16Bit() && TT.isArch32Bit()) ||
+               (T.isArch16Bit() && TT.isArch64Bit()))
----------------
ABataev wrote:
> jhuber6 wrote:
> > ABataev wrote:
> > > Maybe do something like `Triple::getArchPointerBitWidth(T.getArch()) != 
> > > Triple::getArchPointerBitWidth(TT.getArch())"?
> > That was my first thought but that function is private to the Triple class 
> > so I just went with this brute force method. The file has this comment for 
> > the justification.
> > 
> > 
> > ```
> >    /// Test whether the architecture is 64-bit
> >    ///
> >    /// Note that this tests for 64-bit pointer width, and nothing else. Note
> >    /// that we intentionally expose only three predicates, 64-bit, 32-bit, 
> > and
> >    /// 16-bit. The inner details of pointer width for particular 
> > architectures
> >    /// is not summed up in the triple, and so only a coarse grained 
> > predicate
> >    /// system is provided.
> > ```
> Would it be better to convert the results of `isArch...Bit()` to enum and 
> compare enums then? Like:
> ```
> enum {Arch16bit, Arch32bit, Arch64bit} TArch, TTArch;
> 
> if (T.isArch16Bit())
>   Tarch = Arch16Bit;
> else if ...
> ...
> // Same for TT.
> if (TArch != TTArch)
> ...
> ```
You'd end up with a similar situation right, having about 6 conditionals 
manually checking check predicate right?

```
if (T.isArch16Bit())
else if (T.isArch32Bit())
else if (T.isArch64Bit())
if (TT.isArch16Bit())
else if (TT.isArch32Bit())
else if (TT.isArch64Bit())
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88594

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to