This is an archive of the discontinued LLVM Phabricator instance.

[FPEnv] Diagnose pragmas FENV_ROUND,_ACCESS and float_control if target does not support StrictFP
ClosedPublic

Authored by mibintc on Oct 28 2020, 9:32 AM.

Details

Summary

If the target doesn't support setting StrictFP, then ignore pragmas that modify the settings for rounding mode and exception behavior and FENV_ACCESS.
This was requested by @kpn see https://bugs.llvm.org/show_bug.cgi?id=47536
Also @pengfei suggested it as a solution for https://bugs.llvm.org/show_bug.cgi?id=47990

Diff Detail

Event Timeline

mibintc created this revision.Oct 28 2020, 9:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 28 2020, 9:32 AM
mibintc requested review of this revision.Oct 28 2020, 9:32 AM
kpn added a comment.Oct 28 2020, 9:37 AM

I also added "-fexperimental-strict-floating-point" which makes it possible to write tests for a target when bringing up the support on that target. I think it would be confusing for that flag to work on command line arguments but not pragmas.

mibintc updated this revision to Diff 301322.Oct 28 2020, 10:13 AM

Modified according to @kpn suggestion: if the command line has option fexperimental-strict-floating-point, the float pragmas will not be ignored.

In D90316#2359366, @kpn wrote:

I also added "-fexperimental-strict-floating-point" which makes it possible to write tests for a target when bringing up the support on that target. I think it would be confusing for that flag to work on command line arguments but not pragmas.

Thanks, I added this

aaron.ballman added inline comments.Oct 29 2020, 10:22 AM
clang/lib/Parse/ParsePragma.cpp
107

Can you fix the lint issues (run the patch through clang-format)? Same for below.

2969

Can you pass in PragmaName directly, or does that do unfortunate things with single quotes?

clang/test/Parser/pragma-fp-warn.c
12–14

I'd appreciate reversing the order of the comments so that they're in the same order as the pragmas below.

mibintc updated this revision to Diff 301708.Oct 29 2020, 12:21 PM

respond to @aaron.ballman 's review

mibintc marked 2 inline comments as done.Oct 29 2020, 12:23 PM
mibintc added inline comments.
clang/lib/Parse/ParsePragma.cpp
2969

After I changed it to PragmaName, the clang-format didn't find anything to fix

sepavloff accepted this revision.Oct 30 2020, 12:10 AM

LGTM.

clang/test/Parser/pragma-fp-warn.c
2

I would propose to add a line with -triple msp430. This is a simple core, it is unlikely that it gets hardware FP support.

This revision is now accepted and ready to land.Oct 30 2020, 12:10 AM
This revision was landed with ongoing or failed builds.Oct 30 2020, 6:13 AM
This revision was automatically updated to reflect the committed changes.
mibintc marked an inline comment as done.