[llvm] r237376 - TargetSchedule: factor out common code; NFC

[Matthias Braun]

> On May 14, 2015, at 11:26 AM, Eli Bendersky <eliben at google.com> wrote:
> 
> 
> 
> On Thu, May 14, 2015 at 11:01 AM, Matthias Braun <matze at braunis.de <mailto:matze at braunis.de>> wrote:
> Author: matze
> Date: Thu May 14 13:01:13 2015
> New Revision: 237376
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=237376&view=rev <http://llvm.org/viewvc/llvm-project?rev=237376&view=rev>
> Log:
> TargetSchedule: factor out common code; NFC
> 
> Modified:
>     llvm/trunk/include/llvm/CodeGen/TargetSchedule.h
>     llvm/trunk/lib/CodeGen/TargetSchedule.cpp
> 
> Modified: llvm/trunk/include/llvm/CodeGen/TargetSchedule.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/TargetSchedule.h?rev=237376&r1=237375&r2=237376&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/TargetSchedule.h?rev=237376&r1=237375&r2=237376&view=diff>
> ==============================================================================
> --- llvm/trunk/include/llvm/CodeGen/TargetSchedule.h (original)
> +++ llvm/trunk/include/llvm/CodeGen/TargetSchedule.h Thu May 14 13:01:13 2015
> @@ -40,6 +40,9 @@ class TargetSchedModel {
>    SmallVector<unsigned, 16> ResourceFactors;
>    unsigned MicroOpFactor; // Multiply to normalize microops to resource units.
>    unsigned ResourceLCM;   // Resource units per cycle. Latency normalization factor.
> +
> +  unsigned computeInstrLatency(const MCSchedClassDesc &SCDesc) const;
> +
>  public:
>    TargetSchedModel(): SchedModel(MCSchedModel::GetDefaultSchedModel()), STI(nullptr), TII(nullptr) {}
> 
> 
> Modified: llvm/trunk/lib/CodeGen/TargetSchedule.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/TargetSchedule.cpp?rev=237376&r1=237375&r2=237376&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/TargetSchedule.cpp?rev=237376&r1=237375&r2=237376&view=diff>
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/TargetSchedule.cpp (original)
> +++ llvm/trunk/lib/CodeGen/TargetSchedule.cpp Thu May 14 13:01:13 2015
> @@ -224,6 +224,19 @@ unsigned TargetSchedModel::computeOperan
>    return DefMI->isTransient() ? 0 : TII->defaultDefLatency(SchedModel, DefMI);
>  }
> 
> +unsigned
> +TargetSchedModel::computeInstrLatency(const MCSchedClassDesc &SCDesc) const {
> +  unsigned Latency = 0;
> +  for (unsigned DefIdx = 0, DefEnd = SCDesc.NumWriteLatencyEntries;
> +       DefIdx != DefEnd; ++DefIdx) {
> +    // Lookup the definition's write latency in SubtargetInfo.
> +    const MCWriteLatencyEntry *WLEntry =
> +      STI->getWriteLatencyEntry(&SCDesc, DefIdx);
> +    Latency = std::max(Latency, capLatency(WLEntry->Cycles));
> +  }
> +  return Latency;
> +}
> +
>  unsigned TargetSchedModel::computeInstrLatency(unsigned Opcode) const {
>    assert(hasInstrSchedModel() && "Only call this function with a SchedModel");
> 
> @@ -231,16 +244,8 @@ unsigned TargetSchedModel::computeInstrL
>    const MCSchedClassDesc *SCDesc = SchedModel.getSchedClassDesc(SCIdx);
>    unsigned Latency = 0;
> 
> -  if (SCDesc->isValid() && !SCDesc->isVariant()) {
> -    for (unsigned DefIdx = 0, DefEnd = SCDesc->NumWriteLatencyEntries;
> -         DefIdx != DefEnd; ++DefIdx) {
> -      // Lookup the definition's write latency in SubtargetInfo.
> -      const MCWriteLatencyEntry *WLEntry =
> -          STI->getWriteLatencyEntry(SCDesc, DefIdx);
> -      Latency = std::max(Latency, capLatency(WLEntry->Cycles));
> -    }
> -    return Latency;
> -  }
> +  if (SCDesc->isValid() && !SCDesc->isVariant())
> +    return computeInstrLatency(*SCDesc);
> 
>    assert(Latency && "No MI sched latency");
> 
> Hi Mathias,
> 
> This leaves this assert in a weird state, since Latency is not modified from its initialized value of 0. So the assert will always fire. In this case, it should be modified accordingly and Latency removed. 
> 
> Unless there's another issue here that I'm missing....
No I think this is effectively an assert(0), even before my refactoring. I've turned it into an llvm_unreachable.

- Matthias
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150514/4813b184/attachment.html>