This is an archive of the discontinued LLVM Phabricator instance.

process the -fno-signed-zeros optimization flag (PR20870)
ClosedPublic

Authored by spatel on Jan 7 2015, 5:17 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

spatel updated this revision to Diff 17881.Jan 7 2015, 5:17 PM
spatel retitled this revision from to process the -fno-signed-zeros optimization flag (PR20870).
spatel updated this object.
spatel edited the test plan for this revision. (Show Details)
spatel added reviewers: hfinkel, majnemer, chandlerc.
spatel added a subscriber: Unknown Object (MLST).

Ping * 2.

I haven't waded into cfe before this, so if there's someone better to take a look at this patch, please let me know. Thanks!

rsmith added a subscriber: rsmith.Jan 22 2015, 11:20 AM
rsmith added inline comments.
lib/Frontend/CompilerInvocation.cpp
429–430 ↗(On Diff #17881)

We generally try to keep our -cc1 interface simple and minimal. Can we remove the OPT_cl_no_signed_zeros option, and instead create -fno-signed-zeros in the CL case from the driver?

spatel added inline comments.Jan 22 2015, 2:14 PM
lib/Frontend/CompilerInvocation.cpp
429–430 ↗(On Diff #17881)

Sure, that seems reasonable. It looks like we can accomplish this using an Alias in the td file. I'll update the patch.

spatel updated this revision to Diff 18638.Jan 22 2015, 2:19 PM

Changed existing cc1 OpenCL opt for no_unsigned_zeros to be an alias of the driver option for no_unsigned_zeros.

AFAICT, this preserves the outward interface; please let me know if you see any problems.

rsmith accepted this revision.Jan 22 2015, 4:06 PM
rsmith added a reviewer: rsmith.

We have no backwards-compatibility constraints for our -cc1 interface, so we don't need to keep around the -cl-no-signed-zeros flag. Anyway, this patch LGTM

This revision is now accepted and ready to land.Jan 22 2015, 4:06 PM
This revision was automatically updated to reflect the committed changes.

We have no backwards-compatibility constraints for our -cc1 interface, so we don't need to keep around the -cl-no-signed-zeros flag.

Thanks, Richard! I didn't realize we could just nuke that flag. I went ahead and checked in the patch as-is (r226915), but I can get rid of the cl flag in a subsequent patch. Looks like we can do that for a few of those cl FP flags in one patch.