[PATCH] #pragma vectorize

[Aaron Ballman]

On Tue, May 13, 2014 at 3:22 PM, Tyler Nowicki <tnowicki at apple.com> wrote:
> Hi Aaron, Richard,
>
>> After committing r208702/3, your tests now pass for me. I'll take it
>> on faith that the comment and formatting gets resolved when you
>> commit, so LGTM. You should wait for Richard to sign off as well,
>> since he had some comments.
>
>
> Thanks for all the reviewing, I’m glad this is getting closer to being committed!
>
> @Richard, please take a look at the attached patch.
>
>
>> As it's written, it implies that you must specify vectorize or
>> interleave, followed by disable/enable/value, and then optionally
>> supply a value. Eg) #pragma loop vectorize(enable, 4)
>>
>> That's why I think we should have a Union argument type because you
>> really want it to either be enable|disable, or an integer value, not
>> both.
>>
>> Not something that needs doing for this patch by any means. :-)
>
> I’ll do that as a follow-up.

I have ideas on how it should be implemented, so it's on my radar as well.

>
>
>>> I'd be nice to have a command-line switch to turn on and off this pragma.
>>> For example skipping "-fopenmp" makes a compiler to ignore OMP pragmas.
>>> So if you add this switch one can analyse the pure effect of using
>>> vectorizing pragmas at the specified locations.
>>
>> Couldn't that be accomplished via macros already, where you have a
>> macro definition for #pragma loop, and simply noop the macro from the
>> command line with -D?
>
>
> LLVM doesn’t seem to allow you to define a pragma in a macro. It seems to expect the macro to only contain expressions. This is probably a bug?

You have to use _Pragma to accomplish it (C99, IIRC).

>> When you reintroduce your changes, I think I would prefer to leave the
>> iteration in its current (forward) order instead of reverse order (as
>> you have it in your patch). That's simply a bug, and working around it
>> here is likely to ensure that bug sticks around even longer. Once you
>> have introduced your changes, I'd appreciate an extra test case which
>> uses -ast-print and demonstrates the reversed order (I am okay with us
>> XFAILing that test) so that gives us a target test to use to verify
>> the fix when it does happen.
>
> I’ll add the test case in a separate patch since it isn’t related to this work. I’m not really thrilled about the compiler producing pragmas in the reverse order in the mean time though. I updated the patch and tests so they pass with the forward iteration (reverse order) attributes.

Thanks! It's on my radar and I hope to get to it sooner rather than later.

~Aaron