[PATCH] D67631: Add AutoUpgrade function to add new address space datalayout string to existing datalayouts.

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: llvm/trunk/include/llvm/Target/TargetMachine.h:160
   /// check on the validity of this DataLayout.
-  virtual bool isCompatibleDataLayout(const DataLayout &Candidate) const {
+  bool isCompatibleDataLayout(const DataLayout &Candidate) const {
     return DL == Candidate;
----------------
arichardson wrote:
> Why was virtual removed here? We override this in the MIPS backend for our CHERI fork.
> 
> If the intention is to not allow targets to override this, then the documentation also needs to be adjusted.
> However, I doubt avoiding the virtual function call gives any measurable performance speedup.
> 
I dug through the history trying to figure out why it was made virtual, and wasn't able to find any strong reason for it. Obviously, no in tree targets use it. If you want to add it back, I'd approve such a patch.

Also, I seem to recall that overriding this wasn't actually enough for our use cases anyway, so it didn't seem that useful.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67631