[llvm-commits] [PATCH] Asm Loop Nest Comments

[David Greene]

On Saturday 01 August 2009 15:01, Dan Gohman wrote:
> Hi Dave,
>
> This looks good!  Here are some comments:
>
> Instead of using LoopInfo and Loop, this should use
> MachineLoopInfo and MachineLoop.

Ok.  Those didn't exist when I initially did this.  :)

> Would be be practical to compute loop headers just-in-time
> instead of precomputing it and storing it in a map?  Switching
> to MachineLoop would simplify this also.

I'll take a look.

> Would you mind making the default scale be 2 instead of 3?
> That's what LoopInfo's print function uses, for example.  But
> it's not critical.

Sure, no problem.

> I'm a little confused about this:
>
> +  inline Indent indent(int lvl, int amt = 3) {
> +    return(Indent(lvl, amt));
> +  }
>
> This defines a function named indent which wraps a
> constructor for Indent? Why not just have clients invoke
> the class constructor directly?

It's the IO manipulator syntax:

os << indent(3) << string;

It's indent(3) vs. Indent(3).  Ok, not huge but it looks more "standard C++ 
like."  I'll change it.

> Minor style point: return values shouldn't be enclosed in parens.

Gotcha.

                              -Dave