[Patch][LoopInfo] Use CGLoopInfo to generate loop hints

[Hal Finkel]

Hi Tyler,

0001-Rename-Vectorizer-to-Vectorize-and-VectorizeUnroll-t.patch:

  This makes sense; it makes the variable names match the name of the metadata they control. LGTM.

0002-Moved-functionality-from-EmitCondBr-to-CGLoopInfo.patch:

+      ValueInt = static_cast<unsigned>(ValueAPS.getSExtValue());

Do Clang or GCC warn here without the static cast? If not, remove it.

+      case LoopHintAttr::UnrollCount:
+      case LoopHintAttr::VectorizeWidth:
+      case LoopHintAttr::InterleaveCount:
+        break;

The fact that there is no code here for either LoopHintAttr::Disable or case LoopHintAttr::Enable seems counter-intuitive. If this is right, please add a comment explaining why.

--- a/test/CodeGenCXX/pragma-loop.cpp
+++ b/test/CodeGenCXX/pragma-loop.cpp
@@ -10,7 +10,7 @@ void while_test(int *List, int Length) {
 #pragma clang loop vectorize_width(4)
 #pragma clang loop unroll(full)
   while (i < Length) {
-    // CHECK: br i1 {{.*}}, label {{.*}}, label {{.*}}, !llvm.loop ![[LOOP_1:.*]]
+    // CHECK: br label {{.*}}, !llvm.loop ![[LOOP_1:.*]]
     List[i] = i * 2;
     i++;
   }

Why are these things changing? Are we now attaching the metadata to a different branch?

 -Hal

----- Original Message -----
> From: "Tyler Nowicki" <tnowicki at apple.com>
> To: "Hal J. Finkel" <hfinkel at anl.gov>
> Cc: "llvm cfe" <cfe-commits at cs.uiuc.edu>, "Gerolf Hoflehner" <ghoflehner at apple.com>
> Sent: Wednesday, June 24, 2015 6:25:22 PM
> Subject: Re: [Patch][LoopInfo] Use CGLoopInfo to generate loop hints
> 
> 
> 
> Hi Hal,
> 
> Here are the split patches.
> 
> Thanks,
> 
> Tyler
> 
> 
> 
> > On Jun 22, 2015, at 5:40 PM, Hal Finkel <hfinkel at anl.gov> wrote:
> > 
> > Hi Tyler,
> > 
> > - Type::getInt32Ty(Ctx), Attrs.VectorizerWidth))};
> > + Type::getInt32Ty(Ctx), Attrs.VectorizeWidth))};
> > 
> > Could you please factor out the renamings from the functional
> > change?
> >>> 
> >>> Hello,
> >>> 
> >>> I noticed a problem with my implementation of #pragma clang loop
> >>> vectorize(assume_safety). When it was specified on a loop other
> >>> loop hints were lost. The problem is that CGLoopInfo attaches
> >>> metadata a little bit differently than EmitCondBrHints in CGStmt.
> >>> For do-loops CGLoopInfo attaches metadata to the br in the body
> >>> block and for while and for loops, the inc block. EmitCondBrHints
> >>> on the other hand always attaches data to the br in the cond
> >>> block. When specifying assume_safety CGLoopInfo emits an empty
> >>> llvm.loop metadata shadowing the metadata in the cond block. Loop
> >>> transformations like rotate and unswitch would then eliminate the
> >>> cond block.
> >>> 
> >>> This patch unifies both approaches for adding metadata and
> >>> modifies
> >>> the existing safety tests to include non-assume_safety loop
> >>> hints.
> 

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