[PATCH] Support constant expressions, including non-type template parameters, in pragma loop hints

Tyler Nowicki tnowicki at apple.com
Wed Jul 30 10:33:58 PDT 2014


Hi Aaron,

Thanks for the review! Here is the updated patch and responses to some of your comments. The new tests are a couple of lines added to test/Parser/pragma-loop.cpp. This patch is just refactoring in preparation for new features so there really isn’t a need for new tests.

>> +                           ["default", "enable", "disable"],
>> +                           ["Default", "Enable", "Disable"]>,
> 
> This isn't really a "default" so much as "it's not Enabled or
> Disabled", correct? Perhaps UsesValue instead? I know that's not quite
> accurate either since it'll be used for #pragma unroll with no
> arguments…

I’m pretty sure Default or something like it is correct. This value is used for #pragma unroll and #pragma nounroll. The default behavior with unroll enables the unroller and nounroll disables  it. UsesValue would imply that examining the state is required to determine if the Value parameter is used. What I would like to see in the future is for the state and value to be unioned. And the Option would determine if the state or value was needed. AFAIK Tablegen doesn’t have the ability to express unions right now.


>> +    return true;
>> +  }
>> +
>> +  bool StateOption = false;
>> +  if (OptionInfo) // Pragma unroll does not specify an option.
> 
> It doesn't always specify an option, but it sometimes does (otherwise
> there wouldn't be a .Case below).

The naming of the pragmas is unfortunately a little confusing. #pragma unroll and #pragma nounroll don’t have options. There is no need because they only control unrolling. On the other hand #pragma clang loop has options which can be vectorize, interleave, unroll, etc. So when pragma unroll is used we don’t do the check that determines if the option has an argument for the state or value of the loop hint.

#pragma unroll -> enables unrolling
#pragma nounroll -> disables unrolling
#pragma unroll(…) or #pragma unroll … -> specifies the amount of unrolling

The first 2 cases are handled by the early test: if (!Info->HasValue && (PragmaUnroll || PragmaNoUnroll)) { … return true; }

After that we can always assume the 3rd case. So we leave StateOption false and drop down to the part that assumes the argument is a value.

Tyler



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140730/ee64c371/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: loop_hint_attr_state-svn.patch
Type: application/octet-stream
Size: 22737 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140730/ee64c371/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140730/ee64c371/attachment-0001.html>


More information about the cfe-commits mailing list