[PATCH] D36249: Mark tests that need intel 80-bit floats as x86-only

Weiming Zhao via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 3 11:51:05 PDT 2017


weimingz added a comment.

In https://reviews.llvm.org/D36249#830645, @saugustine wrote:

> In https://reviews.llvm.org/D36249#830121, @weimingz wrote:
>
> > I tried to address it via checking pre-defined macros:
> >  https://reviews.llvm.org/D31573
> >
> > As long as the macros are defined correctly by clang, we don't need to worry about the specific target machine. How do you think about it?
>
>
> I like the idea of a feature check, rather than a specific architecture check--that is clearly the right thing to do.
>
> On the other hand, I would like to mark the test as unsupported and not run in that case, rather than running it, saying it passed, but not actually testing anything. That better reflects the state of the implementation. Unfortunately, I don't think that can be done with macro checks. So my preference would be this patch over https://reviews.llvm.org/D31573, but I would also find https://reviews.llvm.org/D31573 acceptable if it came to that.
>
> Finally, 80-bit doubles are a bit of a historical artifact these days. Only x86 and m68k have them (and not even all m68Ks either). So I don't think it matters that much.


I agree that showing the tests pass on architectures that doesn't actually test it is not meaningful. 
This patch LGTM.


Repository:
  rL LLVM

https://reviews.llvm.org/D36249





More information about the cfe-commits mailing list