add getAA in ScheduleDAG for using in HazardRec

[atheel.ma]

add getAA in ScheduleDAGMI for using in HazardRec
add getAA in SchedulePostRATDListfor using in HazardRec

[atrick]

LGTM

[atrick]

If getAA may return NULL, that should be mentioned in the comment.

[atheel.ma]

comment for nullptr return added
added function getAA() to the base class also for cases that you have dynamic pointer of base class ScheduleDAGInstrs that holds SchedulePostRATDList

[atheel.ma]

in SchedulePostRATDList it cant be nullptr because its a pass that intilize the AA in his run

[atheel.ma]

I dont have commit permissions:

Permission to llvm-mirror/llvm.git denied to atheelm

can you push it or tell how to get permissions?

thanks,
Atheel

[atrick]

Atheel,

Just to be clear, you aren't actually using PostRASchedulerList are you?

Once you have contributed a couple sizable patches, you can email Chris Lattner to get commit access.

I could commit this for you, but I just noticed something strange going on. There are two pointers to the current AliasAnalysis, one in ScheduleDAGInstrs and one in ScheduleDAGMI. It looks like they may be set under different conditions. I'm afraid combining them under the same getAA() virtual method may just create more confusion. It looks like Hal was last to work on this.

Hal, can you take a look and commit this in whatever form makes sense to you?

[hfinkel]

In D46801#1099812, @atrick wrote:

Atheel,

Just to be clear, you aren't actually using PostRASchedulerList are you?

Once you have contributed a couple sizable patches, you can email Chris Lattner to get commit access.

I could commit this for you, but I just noticed something strange going on. There are two pointers to the current AliasAnalysis, one in ScheduleDAGInstrs and one in ScheduleDAGMI. It looks like they may be set under different conditions. I'm afraid combining them under the same getAA() virtual method may just create more confusion. It looks like Hal was last to work on this.

Hal, can you take a look and commit this in whatever form makes sense to you?

There's an AAForDep because some targets don't want the cost of doing AA queries during instruction scheduling (or no one feels like tracking down the performance regressions we'd see from enabling it (as would be the case on X86 last we looked at this)).

How would this be used?

[atrick]

@hfinkel This is part of an alternate solution for https://reviews.llvm.org/D46243.

Atheel wants to use AliasAnalysis within the hazard recognizer. I think it's important to avoid adding more implementation classes to headers that will be included/subclassed by out-of-tree targets. But getting the AA pointer from the existing API seems reasonable.

From that discussion:

I’m implementing an optimization pass analysis that will be used from the TargetHazardRecognizer, so I must pass its result in the constructor of the HazaradRecognizer.
I have encountered some open source classes that I need to expand (using inheritance) but couldn’t due to some reasons:

Expanding “MachineSchedulerBase” from MachineScheduler.cpp: I want to inherit it and expand it (for changing the call of CreateTargetPostRAHazardRecognizer)but I can’t because its declared in cpp file not in header the same as above for PostRASchedule pass.

to pass the BasicAAResults when creating the HazaradRecognizer:
llvm::createTargetHazardRecognizer(MachineFunction &MF) (I cant created it inside the HazaradRecognizer because its not a pass)
so I have changed it into:
llvm::createTargetHazardRecognizer(MachineFunction &MF, BasicAAResult *BCA)
and I need to change the functions declaration and implementation that calls it. (functions from (class MachineScheduler : public MachineSchedulerBase ) and more…

[atheel.ma]

thanks Atrick for the explanation,

I do use PostRASchedulerList , anything wrong about it?

[hfinkel]

In D46801#1100340, @atrick wrote:

@hfinkel This is part of an alternate solution for https://reviews.llvm.org/D46243.

Atheel wants to use AliasAnalysis within the hazard recognizer. I think it's important to avoid adding more implementation classes to headers that will be included/subclassed by out-of-tree targets. But getting the AA pointer from the existing API seems reasonable.

Sure, makes sense to me.

From that discussion:

I’m implementing an optimization pass analysis that will be used from the TargetHazardRecognizer, so I must pass its result in the constructor of the HazaradRecognizer.
I have encountered some open source classes that I need to expand (using inheritance) but couldn’t due to some reasons:

Expanding “MachineSchedulerBase” from MachineScheduler.cpp: I want to inherit it and expand it (for changing the call of CreateTargetPostRAHazardRecognizer)but I can’t because its declared in cpp file not in header the same as above for PostRASchedule pass.

to pass the BasicAAResults when creating the HazaradRecognizer:
llvm::createTargetHazardRecognizer(MachineFunction &MF) (I cant created it inside the HazaradRecognizer because its not a pass)
so I have changed it into:
llvm::createTargetHazardRecognizer(MachineFunction &MF, BasicAAResult *BCA)
and I need to change the functions declaration and implementation that calls it. (functions from (class MachineScheduler : public MachineSchedulerBase ) and more…

[atheel.ma]

so Hfinkel, are you going to push it?

include/llvm/CodeGen/MachineScheduler.h

public:
 
   /// If this method returns true, handling of the scheduling regions
   /// themselves (in case of a scheduling boundary in MBB) will be done
   /// beginning with the topmost region of MBB.
   bool doMBBSchedRegionsTopDown() const override {
     return SchedImpl->doMBBSchedRegionsTopDown();
   }
 
+  // Returns AliasAnalysis instance for use in Hazard Recognizer.
+  // The returned AliasAnalysis may be nullptr.
+  AliasAnalysis* getAA() const { return AA; }
+
   // Returns LiveIntervals instance for use in DAG mutators and such.
   LiveIntervals *getLIS() const { return LIS; }
 
   /// Return true if this DAG supports VReg liveness and RegPressure.
   virtual bool hasVRegLiveness() const { return false; }
 
   /// Add a postprocessing step to the DAG builder.
   /// Mutations are applied in the order that they are added after normal DAG

include/llvm/CodeGen/ScheduleDAGInstrs.h

protected:
     AliasAnalysis *AAForDep = nullptr;
 
     /// Remember a generic side-effecting instruction as we proceed.
     /// No other SU ever gets scheduled around it (except in the special
     /// case of a huge region that gets reduced).
     SUnit *BarrierChain = nullptr;
 
   public:
+    // Returns AliasAnalysis instance for use in Hazard Recognizer.
+    // The returned AliasAnalysis may be nullptr.
+    virtual AliasAnalysis* getAA() const { return AAForDep; }
+
     /// A list of SUnits, used in Value2SUsMap, during DAG construction.
     /// Note: to gain speed it might be worth investigating an optimized
     /// implementation of this data structure, such as a singly linked list
     /// with a memory pool (SmallVector was tried but slow and SparseSet is not
     /// applicable).
     using SUList = std::list<SUnit *>;
 
   protected:

lib/CodeGen/PostRASchedulerList.cpp

public:
     /// instruction, which will not be scheduled.
     ///
     void Observe(MachineInstr &MI, unsigned Count);
 
     /// finishBlock - Clean up register live-range state.
     ///
     void finishBlock() override;
 
+    // Returns AliasAnalysis instance for use in Hazard Recognizer.
+    AliasAnalysis* getAA() const { return AA; }
+
   private:
     /// Apply each ScheduleDAGMutation step in order.
     void postprocessDAG();
 
     void ReleaseSucc(SUnit *SU, SDep *SuccEdge);
     void ReleaseSuccessors(SUnit *SU);
     void ScheduleNodeTopDown(SUnit *SU, unsigned CurCycle);
     void ListScheduleTopDown();