Page MenuHomePhabricator

Update clang for D20260

Authored by pcc on May 17 2016, 2:55 PM.

Diff Detail


Event Timeline

pcc updated this revision to Diff 57527.May 17 2016, 2:55 PM
pcc retitled this revision from to Update clang for D20260.
pcc updated this object.
pcc added reviewers: rafael, majnemer.
pcc added a subscriber: cfe-commits.
rsmith added a subscriber: rsmith.May 17 2016, 4:12 PM
rsmith added inline comments.
3 ↗(On Diff #57527)

Where does this come from? It doesn't look like Clang adds this, and this test does not appear to enable any optimizations.

8–9 ↗(On Diff #57527)


4 ↗(On Diff #57527)


pcc added inline comments.May 17 2016, 4:24 PM
3 ↗(On Diff #57527)

OpenCL enables optimizations by default in the frontend, see getOptimizationLevel in lib/Frontend/CompilerInvocation.cpp.

It does seem a little weird that we're doing this in the frontend, this should probably be a driver feature. I must apologise for me-from-5-years-ago who implemented this in r120876.

In the meantime, maybe we should be adding -cl-opt-disable to the OpenCL codegen tests.

rsmith accepted this revision.Jun 14 2016, 2:01 PM
rsmith added a reviewer: rsmith.

Adding -cl-opt-disable to the tests seems fine as an alternative short-term approach; LGTM with that approach (either as part of this change or a separate change before/after). In the long term, I agree that it would be better if the driver specified the relevant -O flag here.

This revision is now accepted and ready to land.Jun 14 2016, 2:01 PM
This revision was automatically updated to reflect the committed changes.
pcc added a comment.Jun 14 2016, 2:11 PM

Thanks, the patch I committed adds -cl-opt-disable to the tests.