[llvm] r199886 - Prevent repetitive warnings for unrecognized processors and features

Artyom Skrobov Artyom.Skrobov at arm.com
Fri Jan 24 09:18:07 PST 2014


>>> +// This needs to be shared between the instantiations of Find<>
>>> +typedef std::pair<std::string,std::string> KeyWithType;
>>> +static SmallSet<KeyWithType,10> reportedAsUnrecognized;
>>>
>>> This is introducing a global into the libraries, which is
>>> verboten. Please revert and find a design that doesn't require
>>> the global.
>>> This should have been caught in pre-commit review.
>>
>> This global is not very global (namely, it's static); besides that,
>> http://llvm.org/docs/CodingStandards.html doesn't include any prohibition
of
>> globals.
>>
>> Should I turn it into a static member of SubtargetFeatures class?
>> This wouldn't reduce neither the lifetime nor the visibility of this
>> variable, but would probably make it less eye-catching.
>
> It's not a question of how eye-catching it is. It's a practical issue
> about how this impacts the reusability of LLVM - eg: two independent
> users of the LLVM library in the same process would have substantial
> problems with this change, would they not? (racing, if nothing else)

David, the very point of this variable is to be shared between independent
callers within the same process, to prevent the same warning from being
reported multiple times from multiple calling points.

I agree that concurrent modifications of the SmallSet can be a concern. I
would very much appreciate if you suggested a solution that improves the
thread-safety while keeping the data shared between the callers.

As an example that "it isn't just me", MCSchedModel::DefaultSchedModel is a
similarly global, mutable static data member, in roughly the same area of
code.







More information about the llvm-commits mailing list