[PATCH] D11167: Avoid passing objects with __declspec(align) members by value (PR24113) - llvm part

Reid Kleckner rnk at google.com
Tue Jul 14 09:21:10 PDT 2015


rnk added inline comments.

================
Comment at: include/llvm/ADT/iterator_range.h:35
@@ -34,4 +34,3 @@
 public:
-  iterator_range(IteratorT begin_iterator, IteratorT end_iterator)
-      : begin_iterator(std::move(begin_iterator)),
-        end_iterator(std::move(end_iterator)) {}
+  iterator_range(const IteratorT &begin_iterator, const IteratorT &end_iterator)
+      : begin_iterator(begin_iterator),
----------------
I'd bug Chandler about this.

================
Comment at: include/llvm/IR/PassManagerInternal.h:313
@@ -312,3 +312,3 @@
 struct AnalysisPassModel<IRUnitT, PassT, false> : AnalysisPassConcept<IRUnitT> {
-  explicit AnalysisPassModel(PassT Pass) : Pass(std::move(Pass)) {}
+  explicit AnalysisPassModel(const PassT &Pass) : Pass(Pass) {}
   // We have to explicitly define all the special member functions because MSVC
----------------
I'd bug Chandler about this.

================
Comment at: include/llvm/Support/AlignOf.h:140-142
@@ -181,1 +139,5 @@
 
+LLVM_ALIGNEDCHARARRAY_TEMPLATE_ALIGNMENT(1)
+LLVM_ALIGNEDCHARARRAY_TEMPLATE_ALIGNMENT(2)
+LLVM_ALIGNEDCHARARRAY_TEMPLATE_ALIGNMENT(4)
+LLVM_ALIGNEDCHARARRAY_TEMPLATE_ALIGNMENT(8)
----------------
So, it occurs to me that MSVC can guarantee 1, 2, or 4 byte stack alignment on x86, just not 8 or more. Does it help reduce the diff if we undo these?

================
Comment at: lib/CodeGen/AsmPrinter/DIEHash.h:137
@@ -136,3 +136,3 @@
   /// \brief Hashes an individual attribute.
-  void hashAttribute(DIEValue Value, dwarf::Tag Tag);
+  void hashAttribute(const DIEValue &Value, dwarf::Tag Tag);
 
----------------
For DIEValue, we could ask Duncan why he used AlignedCharArrayUnion instead of regular union in the first place.

================
Comment at: lib/Target/ARM/AsmParser/ARMAsmParser.cpp:192
@@ -191,3 +191,3 @@
 
-  bool validatetLDMRegList(MCInst Inst, const OperandVector &Operands,
+  bool validatetLDMRegList(const MCInst &Inst, const OperandVector &Operands,
                            unsigned ListNo, bool IsARPop = false);
----------------
The whole ARM asm parser probably didn't meant to pass these by value and can probably be committed as is.


Repository:
  rL LLVM

http://reviews.llvm.org/D11167







More information about the llvm-commits mailing list