Doxygenize the documentation comments

src/IceTargetLoweringX86BaseImpl.h

 //===- subzero/src/IceTargetLoweringX86BaseImpl.h - x86 lowering -*- C++ -*-==//
 //
 //                        The Subzero Code Generator
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the TargetLoweringX86Base class, which
@@ skipping 16 matching lines @@
 #include "IceOperand.h"
 #include "IceRegistersX8632.h"
 #include "IceTargetLoweringX8632.def"
 #include "IceTargetLoweringX8632.h"
 #include "IceUtils.h"
 #include "llvm/Support/MathExtras.h"
 
 namespace Ice {
 namespace X86Internal {
 
-// A helper class to ease the settings of RandomizationPoolingPause
-// to disable constant blinding or pooling for some translation phases.
+/// A helper class to ease the settings of RandomizationPoolingPause
+/// to disable constant blinding or pooling for some translation phases.
 class BoolFlagSaver {
   BoolFlagSaver() = delete;
   BoolFlagSaver(const BoolFlagSaver &) = delete;
   BoolFlagSaver &operator=(const BoolFlagSaver &) = delete;
 
 public:
   BoolFlagSaver(bool &F, bool NewValue) : OldValue(F), Flag(F) { F = NewValue; }
   ~BoolFlagSaver() { Flag = OldValue; }
 
 private:
   const bool OldValue;
   bool &Flag;
 };
 
 template <class MachineTraits> class BoolFoldingEntry {
   BoolFoldingEntry(const BoolFoldingEntry &) = delete;
 
 public:
   BoolFoldingEntry() = default;
   explicit BoolFoldingEntry(Inst *I);
   BoolFoldingEntry &operator=(const BoolFoldingEntry &) = default;
-  // Instr is the instruction producing the i1-type variable of interest.
+  /// Instr is the instruction producing the i1-type variable of interest.
   Inst *Instr = nullptr;
-  // IsComplex is the cached result of BoolFolding::hasComplexLowering(Instr).
+  /// IsComplex is the cached result of BoolFolding::hasComplexLowering(Instr).
   bool IsComplex = false;
-  // IsLiveOut is initialized conservatively to true, and is set to false when
-  // we encounter an instruction that ends Var's live range.  We disable the
-  // folding optimization when Var is live beyond this basic block.  Note that
-  // if liveness analysis is not performed (e.g. in Om1 mode), IsLiveOut will
-  // always be true and the folding optimization will never be performed.
+  /// IsLiveOut is initialized conservatively to true, and is set to false when
+  /// we encounter an instruction that ends Var's live range.  We disable the
+  /// folding optimization when Var is live beyond this basic block.  Note that
+  /// if liveness analysis is not performed (e.g. in Om1 mode), IsLiveOut will
+  /// always be true and the folding optimization will never be performed.
   bool IsLiveOut = true;
   // NumUses counts the number of times Var is used as a source operand in the
   // basic block.  If IsComplex is true and there is more than one use of Var,
   // then the folding optimization is disabled for Var.
   uint32_t NumUses = 0;
 };
 
 template <class MachineTraits> class BoolFolding {
 public:
   enum BoolFoldingProducerKind {
     PK_None,
     PK_Icmp32,
     PK_Icmp64,
     PK_Fcmp,
     PK_Trunc
   };
 
-  // Currently the actual enum values are not used (other than CK_None), but we
-  // go
-  // ahead and produce them anyway for symmetry with the
-  // BoolFoldingProducerKind.
+  /// Currently the actual enum values are not used (other than CK_None), but we
+  /// go
+  /// ahead and produce them anyway for symmetry with the
+  /// BoolFoldingProducerKind.
   enum BoolFoldingConsumerKind { CK_None, CK_Br, CK_Select, CK_Sext, CK_Zext };
 
 private:
   BoolFolding(const BoolFolding &) = delete;
   BoolFolding &operator=(const BoolFolding &) = delete;
 
 public:
   BoolFolding() = default;
   static BoolFoldingProducerKind getProducerKind(const Inst *Instr);
   static BoolFoldingConsumerKind getConsumerKind(const Inst *Instr);
   static bool hasComplexLowering(const Inst *Instr);
   void init(CfgNode *Node);
   const Inst *getProducerFor(const Operand *Opnd) const;
   void dump(const Cfg *Func) const;
 
 private:
-  // Returns true if Producers contains a valid entry for the given VarNum.
+  /// Returns true if Producers contains a valid entry for the given VarNum.
   bool containsValid(SizeT VarNum) const {
     auto Element = Producers.find(VarNum);
     return Element != Producers.end() && Element->second.Instr != nullptr;
   }
   void setInvalid(SizeT VarNum) { Producers[VarNum].Instr = nullptr; }
-  // Producers maps Variable::Number to a BoolFoldingEntry.
+  /// Producers maps Variable::Number to a BoolFoldingEntry.
   std::unordered_map<SizeT, BoolFoldingEntry<MachineTraits>> Producers;
 };
 
 template <class MachineTraits>
 BoolFoldingEntry<MachineTraits>::BoolFoldingEntry(Inst *I)
     : Instr(I), IsComplex(BoolFolding<MachineTraits>::hasComplexLowering(I)) {}
 
 template <class MachineTraits>
 typename BoolFolding<MachineTraits>::BoolFoldingProducerKind
 BoolFolding<MachineTraits>::getProducerKind(const Inst *Instr) {
@@ skipping 32 matching lines @@
       return CK_None;
     case InstCast::Sext:
       return CK_Sext;
     case InstCast::Zext:
       return CK_Zext;
     }
   }
   return CK_None;
 }
 
-// Returns true if the producing instruction has a "complex" lowering
-// sequence.  This generally means that its lowering sequence requires
-// more than one conditional branch, namely 64-bit integer compares
-// and some floating-point compares.  When this is true, and there is
-// more than one consumer, we prefer to disable the folding
-// optimization because it minimizes branches.
+/// Returns true if the producing instruction has a "complex" lowering
+/// sequence.  This generally means that its lowering sequence requires
+/// more than one conditional branch, namely 64-bit integer compares
+/// and some floating-point compares.  When this is true, and there is
+/// more than one consumer, we prefer to disable the folding
+/// optimization because it minimizes branches.
 template <class MachineTraits>
 bool BoolFolding<MachineTraits>::hasComplexLowering(const Inst *Instr) {
   switch (getProducerKind(Instr)) {
   default:
     return false;
   case PK_Icmp64:
     return true;
   case PK_Fcmp:
     return MachineTraits::TableFcmp[llvm::cast<InstFcmp>(Instr)->getCondition()]
                .C2 != CondX86::Br_None;
@@ skipping 422 matching lines @@
 }
 
 // Converts a ConstantInteger32 operand into its constant value, or
 // MemoryOrderInvalid if the operand is not a ConstantInteger32.
 uint64_t getConstantMemoryOrder(Operand *Opnd) {
   if (auto Integer = llvm::dyn_cast<ConstantInteger32>(Opnd))
     return Integer->getValue();
   return Intrinsics::MemoryOrderInvalid;
 }
 
-// Determines whether the dest of a Load instruction can be folded
-// into one of the src operands of a 2-operand instruction.  This is
-// true as long as the load dest matches exactly one of the binary
-// instruction's src operands.  Replaces Src0 or Src1 with LoadSrc if
-// the answer is true.
+/// Determines whether the dest of a Load instruction can be folded
+/// into one of the src operands of a 2-operand instruction.  This is
+/// true as long as the load dest matches exactly one of the binary
+/// instruction's src operands.  Replaces Src0 or Src1 with LoadSrc if
+/// the answer is true.
 bool canFoldLoadIntoBinaryInst(Operand *LoadSrc, Variable *LoadDest,
                                Operand *&Src0, Operand *&Src1) {
   if (Src0 == LoadDest && Src1 != LoadDest) {
     Src0 = LoadSrc;
     return true;
   }
   if (Src0 != LoadDest && Src1 == LoadDest) {
     Src1 = LoadSrc;
     return true;
   }
@@ skipping 216 matching lines @@
       RegisterArg->setName(Func, "home_reg:" + Arg->getName(Func));
     RegisterArg->setRegNum(RegNum);
     RegisterArg->setIsArg();
     Arg->setIsArg(false);
 
     Args[I] = RegisterArg;
     Context.insert(InstAssign::create(Func, Arg, RegisterArg));
   }
 }
 
-// Helper function for addProlog().
-//
-// This assumes Arg is an argument passed on the stack.  This sets the
-// frame offset for Arg and updates InArgsSizeBytes according to Arg's
-// width.  For an I64 arg that has been split into Lo and Hi components,
-// it calls itself recursively on the components, taking care to handle
-// Lo first because of the little-endian architecture.  Lastly, this
-// function generates an instruction to copy Arg into its assigned
-// register if applicable.
+/// Helper function for addProlog().
+///
+/// This assumes Arg is an argument passed on the stack.  This sets the
+/// frame offset for Arg and updates InArgsSizeBytes according to Arg's
+/// width.  For an I64 arg that has been split into Lo and Hi components,
+/// it calls itself recursively on the components, taking care to handle
+/// Lo first because of the little-endian architecture.  Lastly, this
+/// function generates an instruction to copy Arg into its assigned
+/// register if applicable.
 template <class Machine>
 void TargetX86Base<Machine>::finishArgumentLowering(Variable *Arg,
                                                     Variable *FramePtr,
                                                     size_t BasicFrameOffset,
                                                     size_t &InArgsSizeBytes) {
   Variable *Lo = Arg->getLo();
   Variable *Hi = Arg->getHi();
   Type Ty = Arg->getType();
   if (Lo && Hi && Ty == IceType_i64) {
     assert(Lo->getType() != IceType_i64); // don't want infinite recursion
@@ skipping 22 matching lines @@
     // statistics.
     Ctx->statsUpdateFills();
   }
 }
 
 template <class Machine> Type TargetX86Base<Machine>::stackSlotType() {
   return IceType_i32;
 }
 
 template <class Machine> void TargetX86Base<Machine>::addProlog(CfgNode *Node) {
   // Stack frame layout:

[Karl, 2015/07/06 18:08:48]

Not clear if this should be a doxygen comment.

[ascull, 2015/07/06 19:29:09]

If it should be documented then it needs to be moved out of the function. Can
someone more experienced with the project decide what to do here?

[jvoung (off chromium), 2015/07/06 20:07:43]

Let's not doxygenize it for now.

This is explaining how addProlog will lay out the stack, which is mostly a
concern for addProlog. Some of the other code cares about some aspects of the
stack layout (proper alignment, alloca comes last), but necessarily the ordering
(e.g., multi-block/global comes first vs single-block/local).

   //
   // +------------------------+
   // | 1. return address      |
   // +------------------------+
   // | 2. preserved registers |
   // +------------------------+
   // | 3. padding             |
   // +------------------------+
   // | 4. global spill area   |
   // +------------------------+
@@ skipping 223 matching lines @@
     SizeT j = CalleeSaves.size() - i - 1;
     if (j == RegX8632::Reg_ebp && IsEbpBasedFrame)
       continue;
     if (CalleeSaves[j] && RegsUsed[j]) {
       _pop(getPhysicalRegister(j));
     }
   }
 
   if (!Ctx->getFlags().getUseSandboxing())
     return;
-  // Change the original ret instruction into a sandboxed return sequence.
-  // t:ecx = pop
-  // bundle_lock
-  // and t, ~31
-  // jmp *t
-  // bundle_unlock
-  // FakeUse <original_ret_operand>
+  /// Change the original ret instruction into a sandboxed return sequence.

[Karl, 2015/07/06 18:08:49]

This is in the middle of a method. Should it have doxygen comments?

[ascull, 2015/07/06 19:29:09]

Done.

+  /// t:ecx = pop
+  /// bundle_lock
+  /// and t, ~31
+  /// jmp *t
+  /// bundle_unlock
+  /// FakeUse <original_ret_operand>
   const SizeT BundleSize =
       1 << Func->template getAssembler<>()->getBundleAlignLog2Bytes();
   Variable *T_ecx = makeReg(IceType_i32, RegX8632::Reg_ecx);
   _pop(T_ecx);
   _bundle_lock();
   _and(T_ecx, Ctx->getConstantInt32(~(BundleSize - 1)));
   _jmp(T_ecx);
   _bundle_unlock();
   if (RI->getSrcSize()) {
     Variable *RetValue = llvm::cast<Variable>(RI->getSrc(0));
@@ skipping 173 matching lines @@
     // multiple of the required alignment at runtime.
     Variable *T = makeReg(IceType_i32);
     _mov(T, TotalSize);
     _add(T, Ctx->getConstantInt32(Alignment - 1));
     _and(T, Ctx->getConstantInt32(-Alignment));
     _sub(esp, T);
   }
   _mov(Dest, esp);
 }
 
-// Strength-reduce scalar integer multiplication by a constant (for
-// i32 or narrower) for certain constants.  The lea instruction can be
-// used to multiply by 3, 5, or 9, and the lsh instruction can be used
-// to multiply by powers of 2.  These can be combined such that
-// e.g. multiplying by 100 can be done as 2 lea-based multiplies by 5,
-// combined with left-shifting by 2.
+/// Strength-reduce scalar integer multiplication by a constant (for
+/// i32 or narrower) for certain constants.  The lea instruction can be
+/// used to multiply by 3, 5, or 9, and the lsh instruction can be used
+/// to multiply by powers of 2.  These can be combined such that
+/// e.g. multiplying by 100 can be done as 2 lea-based multiplies by 5,
+/// combined with left-shifting by 2.
 template <class Machine>
 bool TargetX86Base<Machine>::optimizeScalarMul(Variable *Dest, Operand *Src0,
                                                int32_t Src1) {
   // Disable this optimization for Om1 and O0, just to keep things
   // simple there.
   if (Ctx->getFlags().getOptLevel() < Opt_1)
     return false;
   Type Ty = Dest->getType();
   Variable *T = nullptr;
   if (Src1 == -1) {
@@ skipping 40 matching lines @@
         ++CountOps;
       ++Count2;
       Src1 /= 2;
     } else {
       return false;
     }
   }
   // Lea optimization only works for i16 and i32 types, not i8.
   if (Ty != IceType_i16 && Ty != IceType_i32 && (Count3 || Count5 || Count9))
     return false;
-  // Limit the number of lea/shl operations for a single multiply, to
-  // a somewhat arbitrary choice of 3.
+  /// Limit the number of lea/shl operations for a single multiply, to

[Karl, 2015/07/06 18:08:49]

Again, this is within a method. Should it have doxygen comments?

[ascull, 2015/07/06 19:29:09]

Done.

+  /// a somewhat arbitrary choice of 3.
   const uint32_t MaxOpsForOptimizedMul = 3;
   if (CountOps > MaxOpsForOptimizedMul)
     return false;
   _mov(T, Src0);
   Constant *Zero = Ctx->getConstantZero(IceType_i32);
   for (uint32_t i = 0; i < Count9; ++i) {
     const uint16_t Shift = 3; // log2(9-1)
     _lea(T, OperandX8632Mem::create(Func, IceType_void, T, Zero, T, Shift));
     _set_dest_nonkillable();
   }
@@ skipping 335 matching lines @@
         // pshufd  T3, Src1, {1,0,3,0}
         // # T1 = {Src0[0] * Src1[0], Src0[2] * Src1[2]}
         // pmuludq T1, Src1
         // # T2 = {Src0[1] * Src1[1], Src0[3] * Src1[3]}
         // pmuludq T2, T3
         // # T1 = {lo(T1[0]), lo(T1[2]), lo(T2[0]), lo(T2[2])}
         // shufps  T1, T2, {0,2,0,2}
         // pshufd  T4, T1, {0,2,1,3}
         // movups  Dest, T4
 
-        // Mask that directs pshufd to create a vector with entries
-        // Src[1, 0, 3, 0]
+        /// Mask that directs pshufd to create a vector with entries

[Karl, 2015/07/06 18:08:48]

Similar question here (not at declaration level).

[ascull, 2015/07/06 19:29:09]

Done.

+        /// Src[1, 0, 3, 0]
         const unsigned Constant1030 = 0x31;
         Constant *Mask1030 = Ctx->getConstantInt32(Constant1030);
-        // Mask that directs shufps to create a vector with entries
-        // Dest[0, 2], Src[0, 2]
+        /// Mask that directs shufps to create a vector with entries
+        /// Dest[0, 2], Src[0, 2]
         const unsigned Mask0202 = 0x88;
-        // Mask that directs pshufd to create a vector with entries
-        // Src[0, 2, 1, 3]
+        /// Mask that directs pshufd to create a vector with entries
+        /// Src[0, 2, 1, 3]
         const unsigned Mask0213 = 0xd8;
         Variable *T1 = makeReg(IceType_v4i32);
         Variable *T2 = makeReg(IceType_v4i32);
         Variable *T3 = makeReg(IceType_v4i32);
         Variable *T4 = makeReg(IceType_v4i32);
         _movp(T1, Src0);
         _pshufd(T2, Src0, Mask1030);
         _pshufd(T3, Src1, Mask1030);
         _pmuludq(T1, Src1);
         _pmuludq(T2, T3);
@@ skipping 589 matching lines @@
       if (DestTy == IceType_v16i8) {
         // onemask = materialize(1,1,...); dst = (src & onemask) > 0
         Variable *OneMask = makeVectorOfOnes(Dest->getType());
         Variable *T = makeReg(DestTy);
         _movp(T, Src0RM);
         _pand(T, OneMask);
         Variable *Zeros = makeVectorOfZeros(Dest->getType());
         _pcmpgt(T, Zeros);
         _movp(Dest, T);
       } else {
-        // width = width(elty) - 1; dest = (src << width) >> width
+        /// width = width(elty) - 1; dest = (src << width) >> width
         SizeT ShiftAmount =
             Traits::X86_CHAR_BIT * typeWidthInBytes(typeElementType(DestTy)) -
             1;
         Constant *ShiftConstant = Ctx->getConstantInt8(ShiftAmount);
         Variable *T = makeReg(DestTy);
         _movp(T, Src0RM);
         _psll(T, ShiftConstant);
         _psra(T, ShiftConstant);
         _movp(Dest, T);
       }
@@ skipping 211 matching lines @@
     if (isVectorType(Dest->getType())) {
       assert(Dest->getType() == IceType_v4f32 &&
              Inst->getSrc(0)->getType() == IceType_v4i32);
       Operand *Src0RM = legalize(Inst->getSrc(0), Legal_Reg | Legal_Mem);
       if (llvm::isa<OperandX8632Mem>(Src0RM))
         Src0RM = legalizeToVar(Src0RM);
       Variable *T = makeReg(Dest->getType());
       _cvt(T, Src0RM, InstX8632Cvt::Dq2ps);
       _movp(Dest, T);
     } else if (Inst->getSrc(0)->getType() == IceType_i64) {
-      // Use a helper for x86-32.
+      /// Use a helper for x86-32.

[Karl, 2015/07/06 18:08:49]

Should this be a doxygen comment (inside method).

[ascull, 2015/07/06 19:29:09]

Done.

       const SizeT MaxSrcs = 1;
       Type DestType = Dest->getType();
       InstCall *Call =
           makeHelperCall(isFloat32Asserting32Or64(DestType) ? H_sitofp_i64_f32
                                                             : H_sitofp_i64_f64,
                          Dest, MaxSrcs);
       // TODO: Call the correct compiler-rt helper function.
       Call->addArg(Inst->getSrc(0));
       lowerCall(Call);
       return;
@@ skipping 15 matching lines @@
     Operand *Src0 = Inst->getSrc(0);
     if (isVectorType(Src0->getType())) {
       assert(Dest->getType() == IceType_v4f32 &&
              Src0->getType() == IceType_v4i32);
       const SizeT MaxSrcs = 1;
       InstCall *Call = makeHelperCall(H_uitofp_4xi32_4xf32, Dest, MaxSrcs);
       Call->addArg(Src0);
       lowerCall(Call);
     } else if (Src0->getType() == IceType_i64 ||
                Src0->getType() == IceType_i32) {
-      // Use a helper for x86-32 and x86-64.  Also use a helper for
-      // i32 on x86-32.
+      /// Use a helper for x86-32 and x86-64.  Also use a helper for
+      /// i32 on x86-32.

[Karl, 2015/07/06 18:08:48]

Again, not a declaration. Should it be ///  ?

[ascull, 2015/07/06 19:29:09]

Done.

       const SizeT MaxSrcs = 1;
       Type DestType = Dest->getType();
       IceString TargetString;
       if (isInt32Asserting32Or64(Src0->getType())) {
         TargetString = isFloat32Asserting32Or64(DestType) ? H_uitofp_i32_f32
                                                           : H_uitofp_i32_f64;
       } else {
         TargetString = isFloat32Asserting32Or64(DestType) ? H_uitofp_i64_f32
                                                           : H_uitofp_i64_f64;
       }
@@ skipping 558 matching lines @@
     }
 
     if (Index == 0) {
       Variable *T = makeReg(Ty);
       _movp(T, SourceVectRM);
       _movss(T, ElementR);
       _movp(Inst->getDest(), T);
       return;
     }
 
-    // shufps treats the source and desination operands as vectors of
-    // four doublewords.  The destination's two high doublewords are
-    // selected from the source operand and the two low doublewords are
-    // selected from the (original value of) the destination operand.
-    // An insertelement operation can be effected with a sequence of two
-    // shufps operations with appropriate masks.  In all cases below,
-    // Element[0] is being inserted into SourceVectOperand.  Indices are
-    // ordered from left to right.
-    //
-    // insertelement into index 1 (result is stored in ElementR):
-    //   ElementR := ElementR[0, 0] SourceVectRM[0, 0]
-    //   ElementR := ElementR[3, 0] SourceVectRM[2, 3]
-    //
-    // insertelement into index 2 (result is stored in T):
-    //   T := SourceVectRM
-    //   ElementR := ElementR[0, 0] T[0, 3]
-    //   T := T[0, 1] ElementR[0, 3]
-    //
-    // insertelement into index 3 (result is stored in T):
-    //   T := SourceVectRM
-    //   ElementR := ElementR[0, 0] T[0, 2]
-    //   T := T[0, 1] ElementR[3, 0]
+    /// shufps treats the source and desination operands as vectors of
+    /// four doublewords.  The destination's two high doublewords are
+    /// selected from the source operand and the two low doublewords are
+    /// selected from the (original value of) the destination operand.
+    /// An insertelement operation can be effected with a sequence of two
+    /// shufps operations with appropriate masks.  In all cases below,
+    /// Element[0] is being inserted into SourceVectOperand.  Indices are
+    /// ordered from left to right.
+    ///
+    /// insertelement into index 1 (result is stored in ElementR):
+    ///   ElementR := ElementR[0, 0] SourceVectRM[0, 0]
+    ///   ElementR := ElementR[3, 0] SourceVectRM[2, 3]
+    ///
+    /// insertelement into index 2 (result is stored in T):
+    ///   T := SourceVectRM
+    ///   ElementR := ElementR[0, 0] T[0, 3]
+    ///   T := T[0, 1] ElementR[0, 3]
+    ///
+    /// insertelement into index 3 (result is stored in T):
+    ///   T := SourceVectRM
+    ///   ElementR := ElementR[0, 0] T[0, 2]
+    ///   T := T[0, 1] ElementR[3, 0]

[Karl, 2015/07/06 18:08:49]

Again, inside method. Should this be /// ?

[ascull, 2015/07/06 19:29:09]

Done.

     const unsigned char Mask1[3] = {0, 192, 128};
     const unsigned char Mask2[3] = {227, 196, 52};
 
     Constant *Mask1Constant = Ctx->getConstantInt32(Mask1[Index - 1]);
     Constant *Mask2Constant = Ctx->getConstantInt32(Mask2[Index - 1]);
 
     if (Index == 1) {
       _shufps(ElementR, SourceVectRM, Mask1Constant);
       _shufps(ElementR, SourceVectRM, Mask2Constant);
       _movp(Inst->getDest(), ElementR);
@@ skipping 654 matching lines @@
   // the end of the loop, since it will be re-used by the loop.
   if (Variable *ValVar = llvm::dyn_cast<Variable>(Val)) {
     Context.insert(InstFakeUse::create(Func, ValVar));
   }
   // The address base (if any) is also reused in the loop.
   if (Variable *Base = Addr->getBase())
     Context.insert(InstFakeUse::create(Func, Base));
   _mov(Dest, T_eax);
 }
 
-// Lowers count {trailing, leading} zeros intrinsic.
-//
-// We could do constant folding here, but that should have
-// been done by the front-end/middle-end optimizations.
+/// Lowers count {trailing, leading} zeros intrinsic.
+///
+/// We could do constant folding here, but that should have
+/// been done by the front-end/middle-end optimizations.
 template <class Machine>
 void TargetX86Base<Machine>::lowerCountZeros(bool Cttz, Type Ty, Variable *Dest,
                                              Operand *FirstVal,
                                              Operand *SecondVal) {
   // TODO(jvoung): Determine if the user CPU supports LZCNT (BMI).
   // Then the instructions will handle the Val == 0 case much more simply
   // and won't require conversion from bit position to number of zeros.
   //
   // Otherwise:
   //   bsr IF_NOT_ZERO, Val
@@ skipping 315 matching lines @@
   lowerAssign(Assign);
 }
 
 template <class Machine> void TargetX86Base<Machine>::doAddressOptLoad() {
   Inst *Inst = Context.getCur();
   Variable *Dest = Inst->getDest();
   Operand *Addr = Inst->getSrc(0);
   Variable *Index = nullptr;
   uint16_t Shift = 0;
   int32_t Offset = 0; // TODO: make Constant
-  // Vanilla ICE load instructions should not use the segment registers,
-  // and computeAddressOpt only works at the level of Variables and Constants,
-  // not other OperandX8632Mem, so there should be no mention of segment
-  // registers there either.
+  /// Vanilla ICE load instructions should not use the segment registers,
+  /// and computeAddressOpt only works at the level of Variables and Constants,
+  /// not other OperandX8632Mem, so there should be no mention of segment
+  /// registers there either.

[Karl, 2015/07/06 18:08:48]

Should this be /// (inside method).

[ascull, 2015/07/06 19:29:09]

Done.

   const OperandX8632Mem::SegmentRegisters SegmentReg =
       OperandX8632Mem::DefaultSegment;
   Variable *Base = llvm::dyn_cast<Variable>(Addr);
   computeAddressOpt(Func, Inst, Base, Index, Shift, Offset);
   if (Base && Addr != Base) {
     Inst->setDeleted();
     Constant *OffsetOp = Ctx->getConstantInt32(Offset);
     Addr = OperandX8632Mem::create(Func, Dest->getType(), Base, OffsetOp, Index,
                                    Shift, SegmentReg);
     Context.insert(InstLoad::create(Func, Dest, Addr));
@@ skipping 207 matching lines @@
 }
 
 template <class Machine> void TargetX86Base<Machine>::doAddressOptStore() {
   InstStore *Inst = llvm::cast<InstStore>(Context.getCur());
   Operand *Data = Inst->getData();
   Operand *Addr = Inst->getAddr();
   Variable *Index = nullptr;
   uint16_t Shift = 0;
   int32_t Offset = 0; // TODO: make Constant
   Variable *Base = llvm::dyn_cast<Variable>(Addr);
-  // Vanilla ICE store instructions should not use the segment registers,
-  // and computeAddressOpt only works at the level of Variables and Constants,
-  // not other OperandX8632Mem, so there should be no mention of segment
-  // registers there either.
+  /// Vanilla ICE store instructions should not use the segment registers,
+  /// and computeAddressOpt only works at the level of Variables and Constants,
+  /// not other OperandX8632Mem, so there should be no mention of segment
+  /// registers there either.

[Karl, 2015/07/06 18:08:48]

Similar here (inside method).

[ascull, 2015/07/06 19:29:10]

Done.

   const OperandX8632Mem::SegmentRegisters SegmentReg =
       OperandX8632Mem::DefaultSegment;
   computeAddressOpt(Func, Inst, Base, Index, Shift, Offset);
   if (Base && Addr != Base) {
     Inst->setDeleted();
     Constant *OffsetOp = Ctx->getConstantInt32(Offset);
     Addr = OperandX8632Mem::create(Func, Data->getType(), Base, OffsetOp, Index,
                                    Shift, SegmentReg);
     InstStore *NewStore = InstStore::create(Func, Data, Addr);
     if (Inst->getDest())
@@ skipping 72 matching lines @@
 
     // Insert the result into position.
     Variable *DestT = Func->template makeVariable(Ty);
     lowerInsertElement(InstInsertElement::create(Func, DestT, T, Res, Index));
     T = DestT;
   }
 
   lowerAssign(InstAssign::create(Func, Dest, T));
 }
 
-// The following pattern occurs often in lowered C and C++ code:
-//
-//   %cmp     = fcmp/icmp pred <n x ty> %src0, %src1
-//   %cmp.ext = sext <n x i1> %cmp to <n x ty>
-//
-// We can eliminate the sext operation by copying the result of pcmpeqd,
-// pcmpgtd, or cmpps (which produce sign extended results) to the result
-// of the sext operation.
+/// The following pattern occurs often in lowered C and C++ code:
+///
+///   %cmp     = fcmp/icmp pred <n x ty> %src0, %src1
+///   %cmp.ext = sext <n x i1> %cmp to <n x ty>
+///
+/// We can eliminate the sext operation by copying the result of pcmpeqd,
+/// pcmpgtd, or cmpps (which produce sign extended results) to the result
+/// of the sext operation.
 template <class Machine>
 void TargetX86Base<Machine>::eliminateNextVectorSextInstruction(
     Variable *SignExtendedResult) {
   if (InstCast *NextCast =
           llvm::dyn_cast_or_null<InstCast>(Context.getNextInst())) {
     if (NextCast->getCastKind() == InstCast::Sext &&
         NextCast->getSrc(0) == SignExtendedResult) {
       NextCast->setDeleted();
       _movp(NextCast->getDest(), legalizeToVar(SignExtendedResult));
       // Skip over the instruction.
@@ skipping 85 matching lines @@
 
 template <class Machine>
 void TargetX86Base<Machine>::lowerOther(const Inst *Instr) {
   if (const auto *RMW = llvm::dyn_cast<InstX8632FakeRMW>(Instr)) {
     lowerRMW(RMW);
   } else {
     TargetLowering::lowerOther(Instr);
   }
 }
 
-// Turn an i64 Phi instruction into a pair of i32 Phi instructions, to
-// preserve integrity of liveness analysis.  Undef values are also
-// turned into zeroes, since loOperand() and hiOperand() don't expect
-// Undef input.
+/// Turn an i64 Phi instruction into a pair of i32 Phi instructions, to
+/// preserve integrity of liveness analysis.  Undef values are also
+/// turned into zeroes, since loOperand() and hiOperand() don't expect
+/// Undef input.
 template <class Machine> void TargetX86Base<Machine>::prelowerPhis() {
   // Pause constant blinding or pooling, blinding or pooling will be done later
   // during phi lowering assignments
   BoolFlagSaver B(RandomizationPoolingPaused, true);
 
   CfgNode *Node = Context.getNode();
   for (Inst &I : Node->getPhis()) {
     auto Phi = llvm::dyn_cast<InstPhi>(&I);
     if (Phi->isDeleted())
       continue;
@@ skipping 25 matching lines @@
   // because they do in fact need a register to materialize the vector
   // of zeroes into.
   if (llvm::isa<ConstantUndef>(Opnd))
     return isScalarFloatingType(Opnd->getType()) ||
            isVectorType(Opnd->getType());
   if (llvm::isa<Constant>(Opnd))
     return isScalarFloatingType(Opnd->getType());
   return true;
 }
 
-// Lower the pre-ordered list of assignments into mov instructions.
-// Also has to do some ad-hoc register allocation as necessary.
+/// Lower the pre-ordered list of assignments into mov instructions.
+/// Also has to do some ad-hoc register allocation as necessary.
 template <class Machine>
 void TargetX86Base<Machine>::lowerPhiAssignments(
     CfgNode *Node, const AssignList &Assignments) {
   // Check that this is a properly initialized shell of a node.
   assert(Node->getOutEdges().size() == 1);
   assert(Node->getInsts().empty());
   assert(Node->getPhis().empty());
   CfgNode *Succ = Node->getOutEdges().front();
   getContext().init(Node);
   // Register set setup similar to regAlloc().
@@ skipping 173 matching lines @@
                                                             int32_t RegNum) {
   assert(Ty == IceType_v4i32 || Ty == IceType_v4f32 || Ty == IceType_v8i16 ||
          Ty == IceType_v16i8);
   if (Ty == IceType_v4f32 || Ty == IceType_v4i32 || Ty == IceType_v8i16) {
     Variable *Reg = makeVectorOfOnes(Ty, RegNum);
     SizeT Shift =
         typeWidthInBytes(typeElementType(Ty)) * Traits::X86_CHAR_BIT - 1;
     _psll(Reg, Ctx->getConstantInt8(Shift));
     return Reg;
   } else {
-    // SSE has no left shift operation for vectors of 8 bit integers.
+    /// SSE has no left shift operation for vectors of 8 bit integers.

[Karl, 2015/07/06 18:08:49]

Why? (inside method).

[ascull, 2015/07/06 19:29:09]

Done.

     const uint32_t HIGH_ORDER_BITS_MASK = 0x80808080;
     Constant *ConstantMask = Ctx->getConstantInt32(HIGH_ORDER_BITS_MASK);
     Variable *Reg = makeReg(Ty, RegNum);
     _movd(Reg, legalize(ConstantMask, Legal_Reg | Legal_Mem));
     _pshufd(Reg, Reg, Ctx->getConstantZero(IceType_i8));
     return Reg;
   }
 }
 
-// Construct a mask in a register that can be and'ed with a
-// floating-point value to mask off its sign bit.  The value will be
-// <4 x 0x7fffffff> for f32 and v4f32, and <2 x 0x7fffffffffffffff>
-// for f64.  Construct it as vector of ones logically right shifted
-// one bit.  TODO(stichnot): Fix the wala TODO above, to represent
-// vector constants in memory.
+/// Construct a mask in a register that can be and'ed with a
+/// floating-point value to mask off its sign bit.  The value will be
+/// <4 x 0x7fffffff> for f32 and v4f32, and <2 x 0x7fffffffffffffff>
+/// for f64.  Construct it as vector of ones logically right shifted
+/// one bit.  TODO(stichnot): Fix the wala TODO above, to represent
+/// vector constants in memory.
 template <class Machine>
 Variable *TargetX86Base<Machine>::makeVectorOfFabsMask(Type Ty,
                                                        int32_t RegNum) {
   Variable *Reg = makeVectorOfMinusOnes(Ty, RegNum);
   _psrl(Reg, Ctx->getConstantInt8(1));
   return Reg;
 }
 
 template <class Machine>
 OperandX8632Mem *
 TargetX86Base<Machine>::getMemoryOperandForStackSlot(Type Ty, Variable *Slot,
                                                      uint32_t Offset) {
   // Ensure that Loc is a stack slot.
   assert(Slot->getWeight().isZero());
   assert(Slot->getRegNum() == Variable::NoRegister);
-  // Compute the location of Loc in memory.
+  /// Compute the location of Loc in memory.

[Karl, 2015/07/06 18:08:49]

Why? (inside method).

[ascull, 2015/07/06 19:29:09]

Done.

   // TODO(wala,stichnot): lea should not be required.  The address of
   // the stack slot is known at compile time (although not until after
   // addProlog()).
   const Type PointerType = IceType_i32;
   Variable *Loc = makeReg(PointerType);
   _lea(Loc, Slot);
   Constant *ConstantOffset = Ctx->getConstantInt32(Offset);
   return OperandX8632Mem::create(Func, Ty, Loc, ConstantOffset);
 }
 
-// Helper for legalize() to emit the right code to lower an operand to a
-// register of the appropriate type.
+/// Helper for legalize() to emit the right code to lower an operand to a
+/// register of the appropriate type.
 template <class Machine>
 Variable *TargetX86Base<Machine>::copyToReg(Operand *Src, int32_t RegNum) {
   Type Ty = Src->getType();
   Variable *Reg = makeReg(Ty, RegNum);
   if (isVectorType(Ty)) {
     _movp(Reg, Src);
   } else {
     _mov(Reg, Src);
   }
   return Reg;
@@ skipping 88 matching lines @@
       NeedsReg = true;
     if (!(Allowed & Legal_Mem) && isScalarFloatingType(Ty))
       // On x86, FP constants are lowered to mem operands.
       NeedsReg = true;
     if (NeedsReg) {
       From = copyToReg(From, RegNum);
     }
     return From;
   }
   if (auto Var = llvm::dyn_cast<Variable>(From)) {
-    // Check if the variable is guaranteed a physical register.  This
-    // can happen either when the variable is pre-colored or when it is
-    // assigned infinite weight.
+    /// Check if the variable is guaranteed a physical register.  This
+    /// can happen either when the variable is pre-colored or when it is
+    /// assigned infinite weight.

[Karl, 2015/07/06 18:08:49]

Why? (inside method).

[ascull, 2015/07/06 19:29:09]

Done.

     bool MustHaveRegister = (Var->hasReg() || Var->getWeight().isInf());
     // We need a new physical register for the operand if:
     //   Mem is not allowed and Var isn't guaranteed a physical
     //   register, or
     //   RegNum is required and Var->getRegNum() doesn't match.
     if ((!(Allowed & Legal_Mem) && !MustHaveRegister) ||
         (RegNum != Variable::NoRegister && RegNum != Var->getRegNum())) {
       From = copyToReg(From, RegNum);
     }
     return From;
   }
   llvm_unreachable("Unhandled operand kind in legalize()");
   return From;
 }
 
-// Provide a trivial wrapper to legalize() for this common usage.
+/// Provide a trivial wrapper to legalize() for this common usage.
 template <class Machine>
 Variable *TargetX86Base<Machine>::legalizeToVar(Operand *From, int32_t RegNum) {
   return llvm::cast<Variable>(legalize(From, Legal_Reg, RegNum));
 }
 
-// For the cmp instruction, if Src1 is an immediate, or known to be a
-// physical register, we can allow Src0 to be a memory operand.
-// Otherwise, Src0 must be copied into a physical register.
-// (Actually, either Src0 or Src1 can be chosen for the physical
-// register, but unfortunately we have to commit to one or the other
-// before register allocation.)
+/// For the cmp instruction, if Src1 is an immediate, or known to be a
+/// physical register, we can allow Src0 to be a memory operand.
+/// Otherwise, Src0 must be copied into a physical register.
+/// (Actually, either Src0 or Src1 can be chosen for the physical
+/// register, but unfortunately we have to commit to one or the other
+/// before register allocation.)
 template <class Machine>
 Operand *TargetX86Base<Machine>::legalizeSrc0ForCmp(Operand *Src0,
                                                     Operand *Src1) {
   bool IsSrc1ImmOrReg = false;
   if (llvm::isa<Constant>(Src1)) {
     IsSrc1ImmOrReg = true;
   } else if (Variable *Var = llvm::dyn_cast<Variable>(Src1)) {
     if (Var->hasReg())
       IsSrc1ImmOrReg = true;
   }
@@ skipping 53 matching lines @@
   inferTwoAddress();
 }
 
 template <class Machine>
 void TargetX86Base<Machine>::makeRandomRegisterPermutation(
     llvm::SmallVectorImpl<int32_t> &Permutation,
     const llvm::SmallBitVector &ExcludeRegisters) const {
   // TODO(stichnot): Declaring Permutation this way loses type/size
   // information.  Fix this in conjunction with the caller-side TODO.
   assert(Permutation.size() >= RegX8632::Reg_NUM);
-  // Expected upper bound on the number of registers in a single
-  // equivalence class.  For x86-32, this would comprise the 8 XMM
-  // registers.  This is for performance, not correctness.
+  /// Expected upper bound on the number of registers in a single
+  /// equivalence class.  For x86-32, this would comprise the 8 XMM
+  /// registers.  This is for performance, not correctness.

[Karl, 2015/07/06 18:08:49]

Why ? (inside method).

[ascull, 2015/07/06 19:29:09]

Done.

   static const unsigned MaxEquivalenceClassSize = 8;
   typedef llvm::SmallVector<int32_t, MaxEquivalenceClassSize> RegisterList;
   typedef std::map<uint32_t, RegisterList> EquivalenceClassMap;
   EquivalenceClassMap EquivalenceClasses;
   SizeT NumShuffled = 0, NumPreserved = 0;
 
 // Build up the equivalence classes of registers by looking at the
 // register properties as well as whether the registers should be
 // explicitly excluded from shuffling.
 #define X(val, encode, name, name16, name8, scratch, preserved, stackptr,      \
@@ skipping 72 matching lines @@
     return;
   Ostream &Str = Ctx->getStrEmit();
   C->emitPoolLabel(Str);
 }
 
 template <class Machine>
 void TargetX86Base<Machine>::emit(const ConstantUndef *) const {
   llvm::report_fatal_error("undef value encountered by emitter.");
 }
 
-// Randomize or pool an Immediate.
+/// Randomize or pool an Immediate.
 template <class Machine>
 Operand *TargetX86Base<Machine>::randomizeOrPoolImmediate(Constant *Immediate,
                                                           int32_t RegNum) {
   assert(llvm::isa<ConstantInteger32>(Immediate) ||
          llvm::isa<ConstantRelocatable>(Immediate));
   if (Ctx->getFlags().getRandomizeAndPoolImmediatesOption() == RPI_None ||
       RandomizationPoolingPaused == true) {
     // Immediates randomization/pooling off or paused
     return Immediate;
   }
@@ skipping 178 matching lines @@
   }
   // the offset is not eligible for blinding or pooling, return the original
   // mem operand
   return MemOperand;
 }
 
 } // end of namespace X86Internal
 } // end of namespace Ice
 
 #endif // SUBZERO_SRC_ICETARGETLOWERINGX86BASEIMPL_H