Page MenuHomePhabricator

[Introduction] C++ Const object optimization in LLVM
Needs ReviewPublic

Authored by lvoufo on Oct 9 2015, 1:32 PM.

Details

Summary

Based on the design doc at
https://docs.google.com/document/d/112O-Q_XrbrU1I4P-oiLCN9u86Qg_BYBdcDsmh7Pn9Nw/edit?usp=sharing,
it handles non-member const objects.
Extensions are discussed in the design doc and will be submitted separately.

Diff Detail

Event Timeline

lvoufo updated this revision to Diff 36985.Oct 9 2015, 1:32 PM
lvoufo retitled this revision from to [Introduction] C++ Const object optimization..
lvoufo updated this object.
lvoufo added a subscriber: cfe-commits.
lvoufo retitled this revision from [Introduction] C++ Const object optimization. to [Introduction] C++ Const object optimization in LLVM.Oct 9 2015, 2:11 PM
lvoufo updated this revision to Diff 36991.Oct 9 2015, 2:18 PM

Clang-formatted.

lvoufo updated this revision to Diff 37003.Oct 9 2015, 3:40 PM

Fix unit test formatting.

lvoufo updated this revision to Diff 37004.Oct 9 2015, 3:42 PM

Fix regression test formatting.

lvoufo updated this revision to Diff 37005.Oct 9 2015, 3:44 PM

All formatting changes should be done by now.

lvoufo updated this object.Oct 9 2015, 7:59 PM
rsmith added a subscriber: rsmith.Dec 2 2015, 5:04 PM

Your D14419 removes a lot of the code added here. Can you update this patch to not add that code? I'd prefer not to review the portion of this code that you're about to delete.

lib/CodeGen/CGDecl.cpp
369

The call to AddInitializerToStaticVarDecl above indirectly calls EmitCXXGlobalVarDeclInit, which already emits an llvm.invariant.start call when appropriate. Please extend the code in EmitCXXGlobalVarDeclInit to handle these new cases (constant types with non-trivial destructors) rather adding another codepath that creates an llvm.invariant.start here.

877–879

I don't think this check is necessary; if we know some region of memory is locally constant, I don't think it matters whether we can trace it back to an alloca.

884

Do we need to add QualType::isWriteOnce for this? CodeGenModule::isTypeConstant seems to be doing mostly the right set of checks. I suggest you add a new flag to it to indicate whether it should check for trivial destructibility, and then use it here instead of using isWriteOnce.

902

The use of RAII here seems unnecessary. You should be able to emit the invariant start and the invariant end cleanup at the same time, by moving this after the call to EmitAutoVarCleanups.

952–958

This conditional should not be necessary: IRBuilder::CreateBitCast should choose between a bitcast constant and a bitcast instruction for you. Just use the non-constant branch in all cases.

959

Not all of our supported versions of MSVC accept this syntax. Create a separate array of llvm::Value * and pass it in here.

lib/CodeGen/CGDeclCXX.cpp
478

This should be unnecessary if you let EmitCXXGlobalVarDeclInit emit the invariant start.

test/CodeGenCXX/init-invariant.cpp
47–48

Please also check that @llvm.invariant.end is called later in the same global initialization function.

51

Likewise for this case.