[PATCH] Add loop unrolling metadata descriptions to LangRef.rst

[Hal Finkel]

----- Original Message -----
> From: "Mark Heffernan" <meheff at google.com>
> To: meheff at google.com, hfinkel at anl.gov
> Cc: llvm-commits at cs.uiuc.edu
> Sent: Friday, July 18, 2014 12:25:31 PM
> Subject: Re: [PATCH] Add loop unrolling metadata descriptions to LangRef.rst
> 
> ================
> Comment at: docs/LangRef.rst:2913
> @@ +2912,3 @@
> +vectorizer will only vectorize loops if it believes it is valid to
> do
> +so.
> +
> ----------------
> hfinkel at anl.gov wrote:
> > You should add a note here that an IR-producer looking at affect
> > this safety determination might find the
> > llvm.mem.parallel_loop_access metadata useful.
> I'll add this.
> 
> ================
> Comment at: docs/LangRef.rst:2957
> @@ +2956,3 @@
> +optimizer believes it is valid to do so.
> +
> +'``llvm.loop.unroll.enable``' Metadata
> ----------------
> hfinkel at anl.gov wrote:
> > We should say something explaining how this is different from
> > llvm.loop.vectorize.unroll (which actually does interleaving --
> > maybe we should rename it?). This is for concatenation unrolling
> > (where the loop body is essentially replicated several times by
> > the unroller, although other optimizations may intermix
> > instructions from different unrolled iterations).
> I think the fix here is to rename llvm.loop.vectorize.unroll to
>  llvm.loop.vectorize.interleave.  Then there is no chance of
> conflating the vectorize metadata with the unroll metadata.  I'll
> work on a patch for this.

Great, thanks!

> 
> ================
> Comment at: docs/LangRef.rst:2965
> @@ +2964,3 @@
> +bit operand value is 0 loop unrolling is disabled. A value of 1
> +indicates that the loop should be fully unrolled. For example:
> +
> ----------------
> hfinkel at anl.gov wrote:
> > I find this confusing. Specifically, to get partial unrolling, what
> > needs to happen? If I want partial unrolling with a specific
> > count, do I need both the 'count' and 'enable' metadata?
> I added a bit more explanation about that case.
> 
> I'd imagine an underlying cause of confusion is that
> ``llvm.loop.unroll.enable 1`` doesn't exactly mean enable.  It means
> try to fully unroll the loop.  So we want the following possible
> hints:
> 
> don't unroll
> unroll fully
> unroll by N
> 
> This doesn't exactly map nicely to the two metadata we have (one with
> boolean operand, one with i32).  Maybe a cleaner way to have done
> this is with the following metadata:
> 
> llvm.loop.unroll.disable (no operand)
> llvm.loop.unroll.fully_unroll (no operand)
> llvm.loop.unroll.count i32
> 
> And only allow a single loop unroll metadata node per loop.  Worth
> making this change?

Yes, I think so. Maybe we can even do it before we branch for the release :-)

 -Hal

> 
> http://reviews.llvm.org/D4576
> 
> 
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory