This patch adds descriptions of the loop unrolling metadata optimization hints ("llvm.loop.unroll.*") to LangRefs.rst. The change also tweaks the language for the vectorization optimization hints to be consistent with the unrolling ones and the language in tools/clang/docs/LanguageExtension.rst.
Details
Diff Detail
Event Timeline
Thanks for working on this!
docs/LangRef.rst | ||
---|---|---|
2913 | 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. | |
2957 | 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). | |
2965 | 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? |
docs/LangRef.rst | ||
---|---|---|
2913 | I'll add this. | |
2957 | 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. | |
2965 | 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 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) And only allow a single loop unroll metadata node per loop. Worth making this change? |
- Original Message -----
From: "Mark Heffernan" <meheff@google.com>
To: meheff@google.com, hfinkel@anl.gov
Cc: llvm-commits@cs.uiuc.edu
Sent: Friday, July 18, 2014 12:25:31 PM
Subject: Re: [PATCH] Add loop unrolling metadata descriptions to LangRef.rstComment at: docs/LangRef.rst:2913
@@ +2912,3 @@
+vectorizer will only vectorize loops if it believes it is valid to
do
+so.+
hfinkel@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@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@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 NThis 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 i32And 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
docs/LangRef.rst | ||
---|---|---|
2965 | I should add that this confusion is also baked into the pragma syntax. The following means unroll fully: #pragma clang loop unroll(enable) |
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.