[PATCH] D135326: Half-done attempt to move tail padding callback from TargetCXXABI to TargetInfo

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.


================
Comment at: clang/include/clang/Basic/TargetInfo.h:953
+  };
+  virtual TailPaddingUseRules getTailPaddingUseRules() const {
+    return UseTailPaddingUnlessPOD03;
----------------
The virtual method works, but it does seem to have caused a behavior change. I would be fine with switches and conditionals on the LLVM triple, or a TargetInfo field set in the constructor. That's how a lot of these flag-like behaviors are controlled. Whatever makes things easy.

My concern was more that we should put the target's rules for tail padding next to the target's rules for what constitutes POD-ness.


================
Comment at: clang/lib/Basic/Targets/OSTargets.h:169
+    if (T.getOS() == llvm::Triple::WatchOS ||
+        this->getCXXABI().getKind() == TargetCXXABI::AppleARM64)
+      return TargetInfo::UseTailPaddingUnlessPOD11;
----------------
I think it would be equivalent to check `T.getArch() == llvm::Triple::AArch64`, that's probably what set the TargetCXXABI.


================
Comment at: clang/lib/Basic/Targets/OSTargets.h:858
+  TargetInfo::TailPaddingUseRules getTailPaddingUseRules() const override {
+    return TargetInfo::AlwaysUseTailPadding;
+  }
----------------
WindowsTargetInfo seems shared with mingw which uses GCC rules, so this isn't quite right. You can check `getTriple().isWindowsMSVCEnvironment()` here to get equivalent behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135326