> + #ifndef LLVM_PARAMETER_ATTRIBUTES_H
> + #define LLVM_PARAMETER_ATTRIBUTES_H
> +
> + #include <llvm/ADT/SmallVector.h>
Please use ""'s instead of <>'s.
> + public:
> + /// Returns the parameter index of a particular parameter
> attribute in this
> + /// list of attributes. Note that the attr_index is an index
> into this
> + /// class's list of attributes, not the index of the
> parameter. The result
> + /// is the index of the parameter.
> + /// @brief Get a parameter index
> + uint16_t getParamIndex(unsigned attr_index) const {
> + return attrs[attr_index].index;
> + }
What is this used for? I'd expect getParamAttrs to be the main
useful api for this class.
> + /// The parameter attributes for the \p indexth parameter are
> returned.
> + /// The 0th parameter refers to the return type of the
> function. Note that
> + /// the \p param_index is an index into the function's
> parameters, not an
> + /// index into this class's list of attributes. The result of
> getParamIndex
> + /// is always suitable input to this function.
> + /// @returns The all the ParameterAttributes for the \p
> indexth parameter
> + /// as a uint16_t of enumeration values OR'd together.
> + /// @brief Get the attributes for a parameter
> + uint16_t getParamAttrs(uint16_t param_index) const;
Did you forget to check in the .cpp file?
> + /// Determines how many parameter attributes are set in this
> ParamAttrsList.
> + /// This says nothing about how many parameters the function
> has. It also
> + /// says nothing about the highest parameter index that has
> attributes.
> + /// @returns the number of parameter attributes in this
> ParamAttrsList.
> + /// @brief Return the number of parameter attributes this
> type has.
> + unsigned size() const { return attrs.size(); }
What is this useful for?
> + /// The set of ParameterAttributes set in Attributes is
> converted to a
> + /// string of equivalent mnemonics. This is, presumably, for
> writing out
> + /// the mnemonics for the assembly writer.
> + /// @brief Convert parameter attribute bits to text
> + static std::string getParamAttrsText(uint16_t Attributes);
This requires #include'ing <string>
> + /// The \p Indexth parameter attribute is converted to string.
> + /// @brief Get the text for the parmeter attributes for one
> parameter.
> + std::string getParamAttrsTextByIndex(uint16_t Index) const {
> + return getParamAttrsText(getParamAttrs(Index));
> + }
I think indexes into the array should be an implementation detail of
the class, not exposed to users.
> + /// @brief Comparison operator for ParamAttrsList
> + bool operator < (const ParamAttrsList& that) const {
> + if (this->attrs.size() < that.attrs.size())
> + return true;
> + if (this->attrs.size() > that.attrs.size())
> + return false;
> + for (unsigned i = 0; i < attrs.size(); ++i) {
> + if (attrs[i].index < that.attrs[i].index)
> + return true;
> + if (attrs[i].index > that.attrs[i].index)
> + return false;
> + if (attrs[i].attrs < that.attrs[i].attrs)
> + return true;
> + if (attrs[i].attrs > that.attrs[i].attrs)
> + return false;
> + }
It seems more straight-forward to implement operator< for smallvector
and ParamAttrsWithIndex.
> + public:
> + /// This adds a pair to the list of parameter index and
> attribute pairs
> + /// represented by this class. No check is made to determine
> whether
> + /// param_index exists already. This pair is just added to
> the end. It is
> + /// the user's responsibility to insert the pairs wisely.
> + /// @brief Insert ParameterAttributes for an index
> + void insert(uint16_t param_index, uint16_t attrs);
I don't like this API. I think the class should take care of merging
attributes for parameters. Also, should this be named "addAttributes
()" or something? Logically this is a bucket of attributes, not a
sequence, so I don't think 'insert' is a good name.
> + SmallVector<ParamAttrsWithIndex,2> attrs; ///< The list of
> attributes
Obviously random/subjective, but I'd suggest bumping this up to
having storage for 4 or 6 attributes.
Thanks for tackling this Reid!
-Chris
_______________________________________________
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
