This is an archive of the discontinued LLVM Phabricator instance.

[clang]Test ensuring -fembed-bitcode passed to cc1 captures pre-opt bitcode.
ClosedPublic

Authored by mtrofin on Sep 22 2020, 1:35 PM.

Details

Summary

This is important to not regress because it allows us to capture pre-optimization
bitcode and options, and replay the full optimization pipeline.

Diff Detail

Event Timeline

mtrofin created this revision.Sep 22 2020, 1:35 PM
mtrofin requested review of this revision.Sep 22 2020, 1:35 PM

I am not sure what exactly is expected here. What is your definition for pre-optimized bitcode and how your test case ensures that? Can you explain a bit more for context?

I am not sure what exactly is expected here. What is your definition for pre-optimized bitcode and how your test case ensures that? Can you explain a bit more for context?

Pre-optimized meaning before the llvm optimization pipeline is called. That's the current implementation, and the test explicitly checks that the inlining of bar into foo doesn't happen.

I could add an "alwaysinline" to bar, to further stress that.

I am not sure what exactly is expected here. What is your definition for pre-optimized bitcode and how your test case ensures that? Can you explain a bit more for context?

Pre-optimized meaning before the llvm optimization pipeline is called. That's the current implementation, and the test explicitly checks that the inlining of bar into foo doesn't happen.

I could add an "alwaysinline" to bar, to further stress that.

I think the current implementation does run optimization passes if the input is c family language and we need to keep it that way (just so that we don't do most of the optimization again). The reason you don't see it running because you are using IR as input. For Apple's implementation, we actually pass -disable-llvm-passes when the input is IR to ensure no optimization passes are running.

I am not sure what exactly is expected here. What is your definition for pre-optimized bitcode and how your test case ensures that? Can you explain a bit more for context?

Pre-optimized meaning before the llvm optimization pipeline is called. That's the current implementation, and the test explicitly checks that the inlining of bar into foo doesn't happen.

I could add an "alwaysinline" to bar, to further stress that.

I think the current implementation does run optimization passes if the input is c family language and we need to keep it that way (just so that we don't do most of the optimization again). The reason you don't see it running because you are using IR as input. For Apple's implementation, we actually pass -disable-llvm-passes when the input is IR to ensure no optimization passes are running.

Afaik, today's implementation has 2 parts: driver and cc1. The cc1 part always emits before opt passes. The driver part, upon seeing -fembed-bitcode, splits compilation in 2 stages. Stage 1 performs ops and emits bc to a file (-emit-llvm). Stage 1 doesn't expose -fembed-bitcode to cc1. Stage 2 takes the output from stage1, disables optimizations, and adds -fembed-bitcode. So together, this gives the semantics you mentioned, but it happens that if you skip the driver and pass -fembed-bitcode to cc1, we get the pre-opt bitcode, which helps my scenario.

Ok, I guess we are on the same page. The idea sounds fine to me.

I would suggest just check that the output matches the input file as much as possible, rather than just check a label and a call instruction.

mtrofin updated this revision to Diff 293761.Sep 23 2020, 9:00 AM

Added a C test, strenghtened the checks

Ok, I guess we are on the same page. The idea sounds fine to me.

I would suggest just check that the output matches the input file as much as possible, rather than just check a label and a call instruction.

Makes sense - also added a C test, same idea.

mtrofin updated this revision to Diff 293762.Sep 23 2020, 9:01 AM

newline at end of file

This revision is now accepted and ready to land.Sep 23 2020, 9:22 AM
This revision was landed with ongoing or failed builds.Sep 23 2020, 9:35 AM
This revision was automatically updated to reflect the committed changes.