[llvm-commits] FW: [llvm] [patch] Porting Hexagon MI Scheduler to new API

[Sebastian Pop]

Hi Sergei,

> Index: lib/Target/Hexagon/HexagonMachineScheduler.cpp
> ===================================================================
> --- lib/Target/Hexagon/HexagonMachineScheduler.cpp	(revision 0)
> +++ lib/Target/Hexagon/HexagonMachineScheduler.cpp	(revision 0)
> @@ -0,0 +1,871 @@
> +//===- HexagonMachineScheduler.cpp - MI Scheduler for Hexagon -------------===//
> +//===----------------------------------------------------------------------===//
> +//
> +// MachineScheduler schedules machine instructions after phi elimination. It
> +// preserves LiveIntervals so it can be invoked before register allocation.
> +//
> +//===----------------------------------------------------------------------===//

I see that you are adding this new file without the UIUC license notice
at the beginning.  Please add this license notice to all new files:

//                     The LLVM Compiler Infrastructure
//
// This file is distributed under the University of Illinois Open Source
// License. See LICENSE.TXT for details.

> Property changes on: lib/Target/Hexagon/HexagonMachineScheduler.h
> ___________________________________________________________________
> Added: svn:executable
>    + *
>
[...]
> Property changes on: lib/Target/Hexagon/HexagonMachineScheduler.cpp
> ___________________________________________________________________
> Added: svn:executable
>    + *

You shouldn't chmod +x these files: please chmod 644 like any other file in the
Hexagon backend.


> Index: lib/Target/Hexagon/HexagonTargetMachine.cpp
[...]
> @@ -69,6 +82,7 @@
>  // Independent Optimization passes to the Pass Manager.
>  bool HexagonTargetMachine::addPassesForOptimizations(PassManagerBase &PM) {
>
> +
>    PM.add(createConstantPropagationPass());
>    PM.add(createLoopSimplifyPass());
>    PM.add(createDeadCodeEliminationPass());
>
[...]
> Index: lib/Target/Hexagon/HexagonRegisterInfo.cpp
> ===================================================================
> --- lib/Target/Hexagon/HexagonRegisterInfo.cpp	(revision 162202)
> +++ lib/Target/Hexagon/HexagonRegisterInfo.cpp	(working copy)
> @@ -44,6 +44,7 @@
>     TII(tii) {
>  }
>
> +
>  const uint16_t* HexagonRegisterInfo::getCalleeSavedRegs(const MachineFunction
>                                                          *MF)
>    const {
> @@ -163,6 +164,7 @@
>
>    const unsigned FrameSize = MFI.getStackSize();
>
> +
>    if (!MFI.hasVarSizedObjects() &&
>        TII.isValidOffset(MI.getOpcode(), (FrameSize+Offset)) &&
>        !TII.isSpillPredRegOp(&MI)) {
> @@ -192,6 +194,7 @@
>            getSubReg(MI.getOperand(0).getReg(), Hexagon::subreg_loreg) :
>            MI.getOperand(0).getReg();
>
> +
>          // Check if offset can fit in addi.
>          if (!TII.isValidOffset(Hexagon::ADD_ri, Offset)) {
>            BuildMI(*MI.getParent(), II, MI.getDebugLoc(),
>

Please remove these changes.

> +/// (ScoreboardHazardRecognizer) and arbitraty target-specific logic.

s/arbitraty/arbitrary/

> +  }
> +  else if (ForceBottomUp) {
[...]
> +  }
> +  else {

These should stand on a single line.

In general, the patch looks great.  Please address these typos, and you'll be
ready to go.

Thanks,
Sebastian
-- 
Qualcomm Innovation Center, Inc is a member of Code Aurora Forum