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

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
444–445

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
444–445

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.