python3kgae marked 4 inline comments as done.
python3kgae added inline comments.
================
Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:208
+ // ctors/dtors added for entry.
+ Triple T(M.getTargetTriple());
+ if (T.getEnvironment() != Triple::EnvironmentType::Library) {
----------------
beanz wrote:
> python3kgae wrote:
> > beanz wrote:
> > > I question whether we should do this early or late. It feels wrong to me
> > > to have clang modifying IR. There are places where clang does this sort
> > > of thing so it isn't unprecedented, but it feels wrong.
> > The reason I want to do it is that with the global variable for
> > ctors/dtors, the ctor/dtor functions will not be removed in optimization
> > passes.
> > As a result, the global variable will have 2 stores ( 1 for the ctor, 1 for
> > the inlined ctor ). That might cause optimizations to go a different path.
> >
> > Also, we insert the call of dtor/ctors to entry which already did what the
> > global variable for ctor/dtor asked. If the global variable for ctor/dtor
> > is still kept, other backends might call the ctor/dtor again.
> That makes sense, and is a fair reasoning. This makes me wonder if we would
> benefit from having a target-specific hook to add early IR optimization
> passes, but that is a whole different discussion...
And a related discussion.
Maybe we should not insert call of ctor/dtor to entries inside a library for
the same reason.
We cannot remove the global variable for ctor/dtor when things need to be
initialized for exported functions other than entries.
If we insert call of ctor to entires, there will be multiple stores for the
global variable( 1 for the ctor, 1 or more for the inlined ctor).
And other backends might call ctor/dtor again when linking.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133993/new/
https://reviews.llvm.org/D133993
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
