[llvm-commits] [PATCH, PR-13709]: Remove unnecessary SRL instruction to access high sub-reg of a register pair.

[Sebastian Pop]

Hi Pranav,

On Mon, Aug 27, 2012 at 1:20 PM, Pranav Bhandarkar
<pranavb at codeaurora.org> wrote:

> +static cl::opt<bool> DisableOptRightShift32("disable-hexagon-optrightshift32",
> +    cl::Hidden, cl::ZeroOrMore, cl::init(false),
> +    cl::desc("Disable Optimization of LSR 32."));

Please remove the dot at the end of a flag description.
Why do we need a flag to disable this transform?
Isn't this always a win?

> +      // %vregDoubleReg1 = LSRd_ri %vregDoubleReg0, 32
> +      // %vregIntReg = COPY %vregDoubleReg1:subreg_loreg.
> +      // and convert into
> +      // %vregIntReg = COPY %VregDoubleReg0:subreg_hireg.

s/VregDoubleReg0/vregDoubleReg0/

> +      if (!DisableOptRightShift32 &&
> +            MI->getOpcode() == Hexagon::LSRd_ri) {

the indentation of this doesn't look correct.

Your patch looks good otherwise. Let's wait a bit for another review.

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