[PATCH] D98773: [X86] Fix compile time regression of D93594.

Nikita Popov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 17 12:53:55 PDT 2021


nikic added inline comments.


================
Comment at: llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp:525
+    auto *DTWP = getAnalysisIfAvailable<DominatorTreeWrapperPass>();
+    auto *DT = DTWP ? &DTWP->getDomTree() : nullptr;
+    auto *LIWP = getAnalysisIfAvailable<LoopInfoWrapperPass>();
----------------
LuoYuanke wrote:
> nikic wrote:
> > I would create a DTU already here and pass it in (it's okay to create a DTU of nullptr). Otherwise the "lazy" strategy will actually be eager, as your DTU object is immediately destroyed.
> Thanks. I'd prefer to not depending to DTU constructor given DT is nullptr, so I create DTU in X86LowerAMXIntrinsics constructor. 
But aren't you still creating a DTU from nullptr, just in a different place now?

I'm okay with your version as well, but it seems more elegant to me to only pass around DTU rather than both DT and DTU. You can then also use it unconditionally, as `DTU->applyUpdates()` is a no-op if there is no actual dominator tree.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98773/new/

https://reviews.llvm.org/D98773



More information about the llvm-commits mailing list