[PATCH 3/3] Clean up multiple minor code issues with attributes implementation

Stephen Lin swlin at post.harvard.edu
Mon Apr 8 11:25:49 PDT 2013


Hi,

I noticed the following code issues, which are addressed by this patch:

1. Verify::VerifyParameterAttrs in "lib/IR/Verifier.cpp" and
AttrBuilder::removeFunctionOnlyAttrs in "lib/IR/Attributes.cpp" (only
called by Verify::VerifyFunctionAttrs) separately maintained a list of
function-only attribute types. I've consolidated the logic into a new
function used for both cases in "lib/IR/Verifier.cpp", so this logic
is in one place (other than the AsmParser front-end)

2. Various functions in "lib/IR/Verifier.cpp" passed AttributeSet
around by reference needlessly, as it's just a handle to an immutable
pimpl body.

3. Semantics of parameters named Index and Idx were inconsistent
between "include/llvm/IR/Attributes.h", "lib/IR/AttributeImpl.h" and
"lib/IR/Attributes.cpp": sometimes these were fixed 1-based indexes of
IR parameters (or AttributeSet::ReturnIndex for IR return values or
AttributeSet::FunctionIndex for IR functions), other times they were
the internal slot for storage in the underlying AttributeSetImpl. I
renamed usage of the former to "Index" and usage of the latter to
"Slot" ("Slot" was already being used consistently for the latter in a
subset of cases)

All tests in "llvm/tests" pass with these changes.

Also, I noticed the following, but did not address it since I wasn't
sure if it was intentional:

4. The parameter and return values that semantically correspond to IR
parameter indices (rather than slots) are sometimes unsigned and
sometimes uint64_t, even though the underlying values are stored as
unsigned. Since some of these functions are explicitly only present
for backward compatibility to code relying on the mask-based
implementation of attributes, I've left them untouched just in case.

Please kindly review this patch and let me know if it can be
committed. (I don't have commit access, so if you could also do that
on my behalf, that would be great!)

Stephen
-------------- next part --------------
A non-text attachment was scrubbed...
Name: clean-verifier.patch
Type: application/octet-stream
Size: 29077 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130408/81d1cbe6/attachment.obj>


More information about the llvm-commits mailing list