[PATCH] D98895: [X86][clang] Disable long double type for -mno-x87 option

[Andrew Savonichev via Phabricator via cfe-commits]

asavonic added inline comments.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:9573
 
-  checkTypeSupport(NewFD->getType(), D.getBeginLoc(), NewFD);
+  if (D.getFunctionDefinitionKind() != FunctionDefinitionKind::Declaration)
+    checkTypeSupport(NewFD->getType(), D.getBeginLoc(), NewFD);
----------------
erichkeane wrote:
> asavonic wrote:
> > erichkeane wrote:
> > > Why are we not checking declarations here?  This doesn't seem right to me.  At least in the offload languages, we still need to check declarations.  Also, if something is a problem with a declaration, why is it not a problem with defaulted/deleted?
> > > Why are we not checking declarations here?  This doesn't seem right to me.  At least in the offload languages, we still need to check declarations.
> > 
> > I assume that if if a function is declared and not used, then it is discarded and no code is generated for it. So it should not really matter if it uses an "unsupported" type.
> > This is important for `long double`, because there are C standard library functions that have `long double` arguments. We skip diagnostics for declarations to avoid errors from standard library headers when the type is actually not used.
> > 
> > > Also, if something is a problem with a declaration, why is it not a problem with defaulted/deleted?
> > 
> > If we can expect that defaulted or deleted functions never result in a code with unsupported types, then we can exclude them as well. Something like this perhaps?
> > ```
> > void no_long_double(long double) = delete;
> > ```
> The problem is that these declarations could be called, right?  So are we catching something like:
> 
> ``` void has_long_double(long double d);
> ....
> has_long_double(1.0); 
> ```
> 
> The deleted types shouldn't result in code being generated, but default will absolutely result in code being generated. Though I guess I can't think of a situation where we would have a defaulted function that could take a `long double` anyway.
> The problem is that these declarations could be called, right?  So are we catching something like:
> 
> ``` void has_long_double(long double d);
> ....
> has_long_double(1.0); 
> ```

Yes, we catch this when the function is called (see below). This is done in `Sema::DiagnoseUseOfDecl`.


> The deleted types shouldn't result in code being generated, but default will absolutely result in code being generated. Though I guess I can't think of a situation where we would have a defaulted function that could take a `long double` anyway.

Thank you. I'll update the patch to exclude deleted functions.



================
Comment at: clang/test/Sema/x86-no-x87.c:35
+#endif
+  return ld_args(x, y);
+}
----------------
This is the case where a declaration is called.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98895