Subzero: Fix over-aggressive bool folding.

src/IceInst.h

 //===- subzero/src/IceInst.h - High-level instructions ----------*- C++ -*-===//
 //
 //                        The Subzero Code Generator
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
@@ skipping 117 matching lines @@
   virtual bool repointEdges(CfgNode *OldNode, CfgNode *NewNode) {
     (void)OldNode;
     (void)NewNode;
     return false;
   }
 
   /// Returns true if the instruction is equivalent to a simple
   /// "var_dest=var_src" assignment where the dest and src are both variables.
   virtual bool isVarAssign() const { return false; }
 
+  /// Returns true if the instruction has a possible side effect of changing
+  /// memory, in which case a memory load should not be reordered with respect
+  /// to this instruction.  We make it return true by default for safety when
+  /// new ICE instructions are defined.
+  virtual bool isMemoryWrite() const { return true; }

[John, 2016/04/21 22:50:01]

what's wrong with = 0, and forcing each new classes to reason about its return
value?

[Jim Stichnoth, 2016/04/21 23:31:35]

Done.

[Jim Stichnoth, 2016/04/21 23:32:41]

Whoops, what I meant was, that would be nice except that g++ doesn't like it
with respect to llvm::ilist<>.

+
   void livenessLightweight(Cfg *Func, LivenessBV &Live);
   /// Calculates liveness for this instruction. Returns true if this instruction
   /// is (tentatively) still live and should be retained, and false if this
   /// instruction is (tentatively) dead and should be deleted. The decision is
   /// tentative until the liveness dataflow algorithm has converged, and then a
   /// separate pass permanently deletes dead instructions.
   bool liveness(InstNumberT InstNumber, LivenessBV &Live, Liveness *Liveness,
                 LiveBeginEndMap *LiveBegin, LiveBeginEndMap *LiveEnd);
 
   /// Get the number of native instructions that this instruction ultimately
@@ skipping 104 matching lines @@
 public:
   static InstAlloca *create(Cfg *Func, Variable *Dest, Operand *ByteCount,
                             uint32_t AlignInBytes) {
     return new (Func->allocate<InstAlloca>())
         InstAlloca(Func, Dest, ByteCount, AlignInBytes);
   }
   uint32_t getAlignInBytes() const { return AlignInBytes; }
   Operand *getSizeInBytes() const { return getSrc(0); }
   bool getKnownFrameOffset() const { return KnownFrameOffset; }
   void setKnownFrameOffset() { KnownFrameOffset = true; }
+  bool isMemoryWrite() const final { return false; }

[Eric Holk, 2016/04/21 22:09:43]

What's the reasoning for making these final?

[John, 2016/04/21 22:50:01]

I tend to prefer using final for classes only. If you think about it, InstAlloca
should be final -- as should every other HLI? Anyway, consider leaving this
non-final.

[Jim Stichnoth, 2016/04/21 22:50:28]

We seem to do that quite regularly in Subzero so I followed suit.  I think John
can approve/disapprove.

[Jim Stichnoth, 2016/04/21 23:31:35]

Done.

   void dump(const Cfg *Func) const override;
   static bool classof(const Inst *Instr) { return Instr->getKind() == Alloca; }
 
 private:
   InstAlloca(Cfg *Func, Variable *Dest, Operand *ByteCount,
              uint32_t AlignInBytes);
 
   const uint32_t AlignInBytes;
   bool KnownFrameOffset = false;
 };
@@ skipping 17 matching lines @@
                                 Operand *Source1, Operand *Source2) {
     return new (Func->allocate<InstArithmetic>())
         InstArithmetic(Func, Op, Dest, Source1, Source2);
   }
   OpKind getOp() const { return Op; }
 
   virtual const char *getInstName() const override;
 
   static const char *getOpName(OpKind Op);
   bool isCommutative() const;
+  bool isMemoryWrite() const final { return false; }
   void dump(const Cfg *Func) const override;
   static bool classof(const Inst *Instr) {
     return Instr->getKind() == Arithmetic;
   }
 
 private:
   InstArithmetic(Cfg *Func, OpKind Op, Variable *Dest, Operand *Source1,
                  Operand *Source2);
 
   const OpKind Op;
 };
 
 /// Assignment instruction. The source operand is captured in getSrc(0). This is
 /// not part of the LLVM bitcode, but is a useful abstraction for some of the
 /// lowering. E.g., if Phi instruction lowering happens before target lowering,
 /// or for representing an Inttoptr instruction, or as an intermediate step for
 /// lowering a Load instruction.
 class InstAssign : public InstHighLevel {
   InstAssign() = delete;
   InstAssign(const InstAssign &) = delete;
   InstAssign &operator=(const InstAssign &) = delete;
 
 public:
   static InstAssign *create(Cfg *Func, Variable *Dest, Operand *Source) {
     return new (Func->allocate<InstAssign>()) InstAssign(Func, Dest, Source);
   }
   bool isVarAssign() const override;
+  bool isMemoryWrite() const final { return false; }
   void dump(const Cfg *Func) const override;
   static bool classof(const Inst *Instr) { return Instr->getKind() == Assign; }
 
 private:
   InstAssign(Cfg *Func, Variable *Dest, Operand *Source);
 };
 
 /// Branch instruction. This represents both conditional and unconditional
 /// branches.
 class InstBr : public InstHighLevel {
@@ skipping 20 matching lines @@
   }
   CfgNode *getTargetTrue() const { return TargetTrue; }
   CfgNode *getTargetFalse() const { return TargetFalse; }
   CfgNode *getTargetUnconditional() const {
     assert(isUnconditional());
     return getTargetFalse();
   }
   NodeList getTerminatorEdges() const override;
   bool isUnconditionalBranch() const override { return isUnconditional(); }
   bool repointEdges(CfgNode *OldNode, CfgNode *NewNode) override;
+  bool isMemoryWrite() const final { return false; }
   void dump(const Cfg *Func) const override;
   static bool classof(const Inst *Instr) { return Instr->getKind() == Br; }
 
 private:
   /// Conditional branch
   InstBr(Cfg *Func, Operand *Source, CfgNode *TargetTrue, CfgNode *TargetFalse);
   /// Unconditional branch
   InstBr(Cfg *Func, CfgNode *Target);
 
   CfgNode *TargetFalse; /// Doubles as unconditional branch target
@@ skipping 60 matching lines @@
   };
 
   static const char *getCastName(OpKind Kind);
 
   static InstCast *create(Cfg *Func, OpKind CastKind, Variable *Dest,
                           Operand *Source) {
     return new (Func->allocate<InstCast>())
         InstCast(Func, CastKind, Dest, Source);
   }
   OpKind getCastKind() const { return CastKind; }
+  bool isMemoryWrite() const final { return false; }
   void dump(const Cfg *Func) const override;
   static bool classof(const Inst *Instr) { return Instr->getKind() == Cast; }
 
 private:
   InstCast(Cfg *Func, OpKind CastKind, Variable *Dest, Operand *Source);
 
   const OpKind CastKind;
 };
 
 /// ExtractElement instruction.
 class InstExtractElement : public InstHighLevel {
   InstExtractElement() = delete;
   InstExtractElement(const InstExtractElement &) = delete;
   InstExtractElement &operator=(const InstExtractElement &) = delete;
 
 public:
   static InstExtractElement *create(Cfg *Func, Variable *Dest, Operand *Source1,
                                     Operand *Source2) {
     return new (Func->allocate<InstExtractElement>())
         InstExtractElement(Func, Dest, Source1, Source2);
   }
 
+  bool isMemoryWrite() const final { return false; }
   void dump(const Cfg *Func) const override;
   static bool classof(const Inst *Instr) {
     return Instr->getKind() == ExtractElement;
   }
 
 private:
   InstExtractElement(Cfg *Func, Variable *Dest, Operand *Source1,
                      Operand *Source2);
 };
 
@@ skipping 11 matching lines @@
 #undef X
         _num
   };
 
   static InstFcmp *create(Cfg *Func, FCond Condition, Variable *Dest,
                           Operand *Source1, Operand *Source2) {
     return new (Func->allocate<InstFcmp>())
         InstFcmp(Func, Condition, Dest, Source1, Source2);
   }
   FCond getCondition() const { return Condition; }
+  bool isMemoryWrite() const final { return false; }
   void dump(const Cfg *Func) const override;
   static bool classof(const Inst *Instr) { return Instr->getKind() == Fcmp; }
 
 private:
   InstFcmp(Cfg *Func, FCond Condition, Variable *Dest, Operand *Source1,
            Operand *Source2);
 
   const FCond Condition;
 };
 
@@ skipping 11 matching lines @@
 #undef X
         _num
   };
 
   static InstIcmp *create(Cfg *Func, ICond Condition, Variable *Dest,
                           Operand *Source1, Operand *Source2) {
     return new (Func->allocate<InstIcmp>())
         InstIcmp(Func, Condition, Dest, Source1, Source2);
   }
   ICond getCondition() const { return Condition; }
+  bool isMemoryWrite() const final { return false; }
   void dump(const Cfg *Func) const override;
   static bool classof(const Inst *Instr) { return Instr->getKind() == Icmp; }
 
 private:
   InstIcmp(Cfg *Func, ICond Condition, Variable *Dest, Operand *Source1,
            Operand *Source2);
 
   const ICond Condition;
 };
 
 /// InsertElement instruction.
 class InstInsertElement : public InstHighLevel {
   InstInsertElement() = delete;
   InstInsertElement(const InstInsertElement &) = delete;
   InstInsertElement &operator=(const InstInsertElement &) = delete;
 
 public:
   static InstInsertElement *create(Cfg *Func, Variable *Dest, Operand *Source1,
                                    Operand *Source2, Operand *Source3) {
     return new (Func->allocate<InstInsertElement>())
         InstInsertElement(Func, Dest, Source1, Source2, Source3);
   }
 
+  bool isMemoryWrite() const final { return false; }
   void dump(const Cfg *Func) const override;
   static bool classof(const Inst *Instr) {
     return Instr->getKind() == InsertElement;
   }
 
 private:
   InstInsertElement(Cfg *Func, Variable *Dest, Operand *Source1,
                     Operand *Source2, Operand *Source3);
 };
 
 /// Call to an intrinsic function. The call target is captured as getSrc(0), and
 /// arg I is captured as getSrc(I+1).
 class InstIntrinsicCall : public InstCall {
   InstIntrinsicCall() = delete;
   InstIntrinsicCall(const InstIntrinsicCall &) = delete;
   InstIntrinsicCall &operator=(const InstIntrinsicCall &) = delete;
 
 public:
   static InstIntrinsicCall *create(Cfg *Func, SizeT NumArgs, Variable *Dest,
                                    Operand *CallTarget,
                                    const Intrinsics::IntrinsicInfo &Info) {
     return new (Func->allocate<InstIntrinsicCall>())
         InstIntrinsicCall(Func, NumArgs, Dest, CallTarget, Info);
   }
   static bool classof(const Inst *Instr) {
     return Instr->getKind() == IntrinsicCall;
   }
 
   Intrinsics::IntrinsicInfo getIntrinsicInfo() const { return Info; }
+  bool isMemoryWrite() const final { return getIntrinsicInfo().IsMemoryWrite; }
 
 private:
   InstIntrinsicCall(Cfg *Func, SizeT NumArgs, Variable *Dest,
                     Operand *CallTarget, const Intrinsics::IntrinsicInfo &Info)
       : InstCall(Func, NumArgs, Dest, CallTarget, false, false,
                  Info.HasSideEffects, Inst::IntrinsicCall),
         Info(Info) {}
 
   const Intrinsics::IntrinsicInfo Info;
 };
 
 /// Load instruction. The source address is captured in getSrc(0).
 class InstLoad : public InstHighLevel {
   InstLoad() = delete;
   InstLoad(const InstLoad &) = delete;
   InstLoad &operator=(const InstLoad &) = delete;
 
 public:
   static InstLoad *create(Cfg *Func, Variable *Dest, Operand *SourceAddr,
                           uint32_t Align = 1) {
     // TODO(kschimpf) Stop ignoring alignment specification.
     (void)Align;
     return new (Func->allocate<InstLoad>()) InstLoad(Func, Dest, SourceAddr);
   }
   Operand *getSourceAddress() const { return getSrc(0); }
+  bool isMemoryWrite() const final { return false; }
   void dump(const Cfg *Func) const override;
   static bool classof(const Inst *Instr) { return Instr->getKind() == Load; }
 
 private:
   InstLoad(Cfg *Func, Variable *Dest, Operand *SourceAddr);
 };
 
 /// Phi instruction. For incoming edge I, the node is Labels[I] and the Phi
 /// source operand is getSrc(I).
 class InstPhi : public InstHighLevel {
   InstPhi() = delete;
   InstPhi(const InstPhi &) = delete;
   InstPhi &operator=(const InstPhi &) = delete;
 
 public:
   static InstPhi *create(Cfg *Func, SizeT MaxSrcs, Variable *Dest) {
     return new (Func->allocate<InstPhi>()) InstPhi(Func, MaxSrcs, Dest);
   }
   void addArgument(Operand *Source, CfgNode *Label);
   Operand *getOperandForTarget(CfgNode *Target) const;
   void clearOperandForTarget(CfgNode *Target);
   CfgNode *getLabel(SizeT Index) const { return Labels[Index]; }
   void setLabel(SizeT Index, CfgNode *Label) { Labels[Index] = Label; }
   void livenessPhiOperand(LivenessBV &Live, CfgNode *Target,
                           Liveness *Liveness);
   Inst *lower(Cfg *Func);
+  bool isMemoryWrite() const final { return false; }
   void dump(const Cfg *Func) const override;
   static bool classof(const Inst *Instr) { return Instr->getKind() == Phi; }
 
 private:
   InstPhi(Cfg *Func, SizeT MaxSrcs, Variable *Dest);
   void destroy(Cfg *Func) override {
     Func->deallocateArrayOf<CfgNode *>(Labels);
     Inst::destroy(Func);
   }
 
@@ skipping 14 matching lines @@
 public:
   static InstRet *create(Cfg *Func, Operand *RetValue = nullptr) {
     return new (Func->allocate<InstRet>()) InstRet(Func, RetValue);
   }
   bool hasRetValue() const { return getSrcSize(); }
   Operand *getRetValue() const {
     assert(hasRetValue());
     return getSrc(0);
   }
   NodeList getTerminatorEdges() const override { return NodeList(); }
+  bool isMemoryWrite() const final { return false; }
   void dump(const Cfg *Func) const override;
   static bool classof(const Inst *Instr) { return Instr->getKind() == Ret; }
 
 private:
   InstRet(Cfg *Func, Operand *RetValue);
 };
 
 /// Select instruction.  The condition, true, and false operands are captured.
 class InstSelect : public InstHighLevel {
   InstSelect() = delete;
   InstSelect(const InstSelect &) = delete;
   InstSelect &operator=(const InstSelect &) = delete;
 
 public:
   static InstSelect *create(Cfg *Func, Variable *Dest, Operand *Condition,
                             Operand *SourceTrue, Operand *SourceFalse) {
     return new (Func->allocate<InstSelect>())
         InstSelect(Func, Dest, Condition, SourceTrue, SourceFalse);
   }
   Operand *getCondition() const { return getSrc(0); }
   Operand *getTrueOperand() const { return getSrc(1); }
   Operand *getFalseOperand() const { return getSrc(2); }
+  bool isMemoryWrite() const final { return false; }
   void dump(const Cfg *Func) const override;
   static bool classof(const Inst *Instr) { return Instr->getKind() == Select; }
 
 private:
   InstSelect(Cfg *Func, Variable *Dest, Operand *Condition, Operand *Source1,
              Operand *Source2);
 };
 
 /// Store instruction. The address operand is captured, along with the data
 /// operand to be stored into the address.
@@ skipping 39 matching lines @@
     assert(I < NumCases);
     return Values[I];
   }
   CfgNode *getLabel(SizeT I) const {
     assert(I < NumCases);
     return Labels[I];
   }
   void addBranch(SizeT CaseIndex, uint64_t Value, CfgNode *Label);
   NodeList getTerminatorEdges() const override;
   bool repointEdges(CfgNode *OldNode, CfgNode *NewNode) override;
+  bool isMemoryWrite() const final { return false; }
   void dump(const Cfg *Func) const override;
   static bool classof(const Inst *Instr) { return Instr->getKind() == Switch; }
 
 private:
   InstSwitch(Cfg *Func, SizeT NumCases, Operand *Source, CfgNode *LabelDefault);
   void destroy(Cfg *Func) override {
     Func->deallocateArrayOf<uint64_t>(Values);
     Func->deallocateArrayOf<CfgNode *>(Labels);
     Inst::destroy(Func);
   }
 
   CfgNode *LabelDefault;
   SizeT NumCases;   /// not including the default case
   uint64_t *Values; /// size is NumCases
   CfgNode **Labels; /// size is NumCases
 };
 
 /// Unreachable instruction. This is a terminator instruction with no operands.
 class InstUnreachable : public InstHighLevel {
   InstUnreachable() = delete;
   InstUnreachable(const InstUnreachable &) = delete;
   InstUnreachable &operator=(const InstUnreachable &) = delete;
 
 public:
   static InstUnreachable *create(Cfg *Func) {
     return new (Func->allocate<InstUnreachable>()) InstUnreachable(Func);
   }
   NodeList getTerminatorEdges() const override { return NodeList(); }
+  bool isMemoryWrite() const final { return false; }
   void dump(const Cfg *Func) const override;
   static bool classof(const Inst *Instr) {
     return Instr->getKind() == Unreachable;
   }
 
 private:
   explicit InstUnreachable(Cfg *Func);
 };
 
 /// BundleLock instruction.  There are no operands. Contains an option
 /// indicating whether align_to_end is specified.
 class InstBundleLock : public InstHighLevel {
   InstBundleLock() = delete;
   InstBundleLock(const InstBundleLock &) = delete;
   InstBundleLock &operator=(const InstBundleLock &) = delete;
 
 public:
   enum Option { Opt_None, Opt_AlignToEnd, Opt_PadToEnd };
   static InstBundleLock *create(Cfg *Func, Option BundleOption) {
     return new (Func->allocate<InstBundleLock>())
         InstBundleLock(Func, BundleOption);
   }
   void emit(const Cfg *Func) const override;
   void emitIAS(const Cfg * /* Func */) const override {}
+  bool isMemoryWrite() const final { return false; }
   void dump(const Cfg *Func) const override;
   Option getOption() const { return BundleOption; }
   static bool classof(const Inst *Instr) {
     return Instr->getKind() == BundleLock;
   }
 
 private:
   Option BundleOption;
   InstBundleLock(Cfg *Func, Option BundleOption);
 };
 
 /// BundleUnlock instruction. There are no operands.
 class InstBundleUnlock : public InstHighLevel {
   InstBundleUnlock() = delete;
   InstBundleUnlock(const InstBundleUnlock &) = delete;
   InstBundleUnlock &operator=(const InstBundleUnlock &) = delete;
 
 public:
   static InstBundleUnlock *create(Cfg *Func) {
     return new (Func->allocate<InstBundleUnlock>()) InstBundleUnlock(Func);
   }
   void emit(const Cfg *Func) const override;
   void emitIAS(const Cfg * /* Func */) const override {}
+  bool isMemoryWrite() const final { return false; }
   void dump(const Cfg *Func) const override;
   static bool classof(const Inst *Instr) {
     return Instr->getKind() == BundleUnlock;
   }
 
 private:
   explicit InstBundleUnlock(Cfg *Func);
 };
 
 /// FakeDef instruction. This creates a fake definition of a variable, which is
@@ skipping 12 matching lines @@
   InstFakeDef(const InstFakeDef &) = delete;
   InstFakeDef &operator=(const InstFakeDef &) = delete;
 
 public:
   static InstFakeDef *create(Cfg *Func, Variable *Dest,
                              Variable *Src = nullptr) {
     return new (Func->allocate<InstFakeDef>()) InstFakeDef(Func, Dest, Src);
   }
   void emit(const Cfg *Func) const override;
   void emitIAS(const Cfg * /* Func */) const override {}
+  bool isMemoryWrite() const final { return false; }
   void dump(const Cfg *Func) const override;
   static bool classof(const Inst *Instr) { return Instr->getKind() == FakeDef; }
 
 private:
   InstFakeDef(Cfg *Func, Variable *Dest, Variable *Src);
 };
 
 /// FakeUse instruction. This creates a fake use of a variable, to keep the
 /// instruction that produces that variable from being dead-code eliminated.
 /// This is useful in a variety of lowering situations. The FakeUse instruction
 /// has no dest, so it can itself never be dead-code eliminated.  A weight can
 /// be provided to provide extra bias to the register allocator - for simplicity
 /// of implementation, weight=N is handled by holding N copies of the variable
 /// as source operands.
 class InstFakeUse : public InstHighLevel {
   InstFakeUse() = delete;
   InstFakeUse(const InstFakeUse &) = delete;
   InstFakeUse &operator=(const InstFakeUse &) = delete;
 
 public:
   static InstFakeUse *create(Cfg *Func, Variable *Src, uint32_t Weight = 1) {
     return new (Func->allocate<InstFakeUse>()) InstFakeUse(Func, Src, Weight);
   }
   void emit(const Cfg *Func) const override;
   void emitIAS(const Cfg * /* Func */) const override {}
+  bool isMemoryWrite() const final { return false; }
   void dump(const Cfg *Func) const override;
   static bool classof(const Inst *Instr) { return Instr->getKind() == FakeUse; }
 
 private:
   InstFakeUse(Cfg *Func, Variable *Src, uint32_t Weight);
 };
 
 /// FakeKill instruction. This "kills" a set of variables by modeling a trivial
 /// live range at this instruction for each (implicit) variable. The primary use
 /// is to indicate that scratch registers are killed after a call, so that the
 /// register allocator won't assign a scratch register to a variable whose live
 /// range spans a call.
 ///
 /// The FakeKill instruction also holds a pointer to the instruction that kills
 /// the set of variables, so that if that linked instruction gets dead-code
 /// eliminated, the FakeKill instruction will as well.
 class InstFakeKill : public InstHighLevel {
   InstFakeKill() = delete;
   InstFakeKill(const InstFakeKill &) = delete;
   InstFakeKill &operator=(const InstFakeKill &) = delete;
 
 public:
   static InstFakeKill *create(Cfg *Func, const Inst *Linked) {
     return new (Func->allocate<InstFakeKill>()) InstFakeKill(Func, Linked);
   }
   const Inst *getLinked() const { return Linked; }
   void emit(const Cfg *Func) const override;
   void emitIAS(const Cfg * /* Func */) const override {}
+  bool isMemoryWrite() const final { return false; }

[Eric Holk, 2016/04/21 22:09:43]

Since all the other instructions have isMemoryWrite() declared final, would it
make sense to add one for stores that returns true?

[Jim Stichnoth, 2016/04/21 22:50:28]

Done.

   void dump(const Cfg *Func) const override;
   static bool classof(const Inst *Instr) {
     return Instr->getKind() == FakeKill;
   }
 
 private:
   InstFakeKill(Cfg *Func, const Inst *Linked);
 
   /// This instruction is ignored if Linked->isDeleted() is true.
   const Inst *Linked;
@@ skipping 16 matching lines @@
     assert(TargetIndex < NumTargets);
     Targets[TargetIndex] = Target;
   }
   bool repointEdges(CfgNode *OldNode, CfgNode *NewNode) override;
   SizeT getId() const { return Id; }
   SizeT getNumTargets() const { return NumTargets; }
   CfgNode *getTarget(SizeT I) const {
     assert(I < NumTargets);
     return Targets[I];
   }
+  bool isMemoryWrite() const final { return false; }
   void dump(const Cfg *Func) const override;
   static bool classof(const Inst *Instr) {
     return Instr->getKind() == JumpTable;
   }
   // Creates a JumpTableData struct (used for ELF emission) that represents this
   // InstJumpTable.
   JumpTableData toJumpTableData(Assembler *Asm) const;
 
   // InstJumpTable is just a placeholder for the switch targets, and it does not
   // need to emit any code, so we redefine emit and emitIAS to do nothing.
@@ skipping 85 matching lines @@
   static void noteHead(Ice::Inst *, Ice::Inst *) {}
   void deleteNode(Ice::Inst *) {}
 
 private:
   mutable ilist_half_node<Ice::Inst> Sentinel;
 };
 
 } // end of namespace llvm
 
 #endif // SUBZERO_SRC_ICEINST_H