[llvm] r254805 - [EarlyCSE] IsSimple vs IsVolatile naming clarification (NFC)

Sean Silva via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 4 18:16:22 PST 2015


On Fri, Dec 4, 2015 at 4:18 PM, Philip Reames via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: reames
> Date: Fri Dec  4 18:18:33 2015
> New Revision: 254805
>
> URL: http://llvm.org/viewvc/llvm-project?rev=254805&view=rev
> Log:
> [EarlyCSE] IsSimple vs IsVolatile naming clarification (NFC)
>
> When the notion of target specific memory intrinsics was introduced to
> EarlyCSE, the commit confused the notions of volatile and simple memory
> access.  Since I'm about to start working on this area, cleanup the naming
> so that patches aren't horribly confusing.  Note that the actual
> implementation was always bailing if the load or store wasn't simple.
>
> Reminder:
> - "volatile" - C++ volatile, can't remove any memory operations, but in
> principal unordered
> - "ordered" - imposes ordering constraints on other nearby memory
> operations
> - "atomic" - can't be split or sheared.  In LLVM terms, all "ordered"
> operations are also atomic so the predicate "isAtomic" is often used.
>

What does "sheared" mean in this context? Widened?

-- Sean Silva


> - "simple" - a load which is none of the above.  These are normal loads
> and what most of the optimizer works with.
>
>
> Modified:
>     llvm/trunk/include/llvm/Analysis/TargetTransformInfo.h
>     llvm/trunk/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
>     llvm/trunk/lib/Transforms/Scalar/EarlyCSE.cpp
>
> Modified: llvm/trunk/include/llvm/Analysis/TargetTransformInfo.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/TargetTransformInfo.h?rev=254805&r1=254804&r2=254805&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/Analysis/TargetTransformInfo.h (original)
> +++ llvm/trunk/include/llvm/Analysis/TargetTransformInfo.h Fri Dec  4
> 18:18:33 2015
> @@ -42,11 +42,13 @@ class Value;
>  /// \brief Information about a load/store intrinsic defined by the target.
>  struct MemIntrinsicInfo {
>    MemIntrinsicInfo()
> -      : ReadMem(false), WriteMem(false), Vol(false), MatchingId(0),
> +      : ReadMem(false), WriteMem(false), IsSimple(false), MatchingId(0),
>          NumMemRefs(0), PtrVal(nullptr) {}
>    bool ReadMem;
>    bool WriteMem;
> -  bool Vol;
> +  /// True only if this memory operation is non-volatile, non-atomic, and
> +  /// unordered.  (See LoadInst/StoreInst for details on each)
> +  bool IsSimple;
>    // Same Id is set by the target for corresponding load/store intrinsics.
>    unsigned short MatchingId;
>    int NumMemRefs;
>
> Modified: llvm/trunk/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64TargetTransformInfo.cpp?rev=254805&r1=254804&r2=254805&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Target/AArch64/AArch64TargetTransformInfo.cpp (original)
> +++ llvm/trunk/lib/Target/AArch64/AArch64TargetTransformInfo.cpp Fri Dec
> 4 18:18:33 2015
> @@ -538,7 +538,7 @@ bool AArch64TTIImpl::getTgtMemIntrinsic(
>    case Intrinsic::aarch64_neon_ld4:
>      Info.ReadMem = true;
>      Info.WriteMem = false;
> -    Info.Vol = false;
> +    Info.IsSimple = true;
>      Info.NumMemRefs = 1;
>      Info.PtrVal = Inst->getArgOperand(0);
>      break;
> @@ -547,7 +547,7 @@ bool AArch64TTIImpl::getTgtMemIntrinsic(
>    case Intrinsic::aarch64_neon_st4:
>      Info.ReadMem = false;
>      Info.WriteMem = true;
> -    Info.Vol = false;
> +    Info.IsSimple = true;
>      Info.NumMemRefs = 1;
>      Info.PtrVal = Inst->getArgOperand(Inst->getNumArgOperands() - 1);
>      break;
>
> Modified: llvm/trunk/lib/Transforms/Scalar/EarlyCSE.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/EarlyCSE.cpp?rev=254805&r1=254804&r2=254805&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/EarlyCSE.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/EarlyCSE.cpp Fri Dec  4 18:18:33 2015
> @@ -388,8 +388,8 @@ private:
>    class ParseMemoryInst {
>    public:
>      ParseMemoryInst(Instruction *Inst, const TargetTransformInfo &TTI)
> -        : Load(false), Store(false), Vol(false), MayReadFromMemory(false),
> -          MayWriteToMemory(false), MatchingId(-1), Ptr(nullptr) {
> +      : Load(false), Store(false), IsSimple(true),
> MayReadFromMemory(false),
> +        MayWriteToMemory(false), MatchingId(-1), Ptr(nullptr) {
>        MayReadFromMemory = Inst->mayReadFromMemory();
>        MayWriteToMemory = Inst->mayWriteToMemory();
>        if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(Inst)) {
> @@ -402,22 +402,22 @@ private:
>            MatchingId = Info.MatchingId;
>            MayReadFromMemory = Info.ReadMem;
>            MayWriteToMemory = Info.WriteMem;
> -          Vol = Info.Vol;
> +          IsSimple = Info.IsSimple;
>            Ptr = Info.PtrVal;
>          }
>        } else if (LoadInst *LI = dyn_cast<LoadInst>(Inst)) {
>          Load = true;
> -        Vol = !LI->isSimple();
> +        IsSimple = LI->isSimple();
>          Ptr = LI->getPointerOperand();
>        } else if (StoreInst *SI = dyn_cast<StoreInst>(Inst)) {
>          Store = true;
> -        Vol = !SI->isSimple();
> +        IsSimple = SI->isSimple();
>          Ptr = SI->getPointerOperand();
>        }
>      }
>      bool isLoad() const { return Load; }
>      bool isStore() const { return Store; }
> -    bool isVolatile() const { return Vol; }
> +    bool isSimple() const { return IsSimple; }
>      bool isMatchingMemLoc(const ParseMemoryInst &Inst) const {
>        return Ptr == Inst.Ptr && MatchingId == Inst.MatchingId;
>      }
> @@ -430,7 +430,7 @@ private:
>    private:
>      bool Load;
>      bool Store;
> -    bool Vol;
> +    bool IsSimple;
>      bool MayReadFromMemory;
>      bool MayWriteToMemory;
>      // For regular (non-intrinsic) loads/stores, this is set to -1. For
> @@ -554,8 +554,8 @@ bool EarlyCSE::processNode(DomTreeNode *
>      ParseMemoryInst MemInst(Inst, TTI);
>      // If this is a non-volatile load, process it.
>      if (MemInst.isValid() && MemInst.isLoad()) {
> -      // Ignore volatile loads.
> -      if (MemInst.isVolatile()) {
> +      // Ignore volatile or ordered loads.
> +      if (!MemInst.isSimple()) {
>          LastStore = nullptr;
>          // Don't CSE across synchronization boundaries.
>          if (Inst->mayWriteToMemory())
> @@ -662,8 +662,8 @@ bool EarlyCSE::processNode(DomTreeNode *
>              MemInst.getPtr(),
>              LoadValue(Inst, CurrentGeneration, MemInst.getMatchingId()));
>
> -        // Remember that this was the last store we saw for DSE.
> -        if (!MemInst.isVolatile())
> +        // Remember that this was the last normal store we saw for DSE.
> +        if (MemInst.isSimple())
>            LastStore = Inst;
>        }
>      }
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151204/8ec2656b/attachment.html>


More information about the llvm-commits mailing list