rjmccall added inline comments.
================ Comment at: include/clang/Basic/Attr.td:1191 +def MipsLongCall : InheritableAttr, TargetSpecificAttr<TargetMips> { + let Spellings = [GCC<"long_call">, GCC<"far">, GCC<"near">]; ---------------- Because this is used for all three attributes, I think you should call it something more general. Perhaps MipsCallStyle? ================ Comment at: include/clang/Basic/Attr.td:1195 + let Accessors = [Accessor<"longCall", [GCC<"long_call">, GCC<"far">]>, + Accessor<"nearCall", [GCC<"near">]>]; + let Documentation = [MipsLongCallDocs]; ---------------- This is not the standard naming convention for accessors. I suggest isLongCall() and isNearCall(). ================ Comment at: include/clang/Basic/AttrDocs.td:1336 +if code compiled using ``-mlong-calls`` switch, it forces compiler to use +the ``jal`` instruction to call the function. + }]; ---------------- I suggest the following wording: Clang supports the ``__attribute__((long_call))``, ``__attribute__((far))``, and ``__attribute__((near))`` attributes on MIPS targets. These attributes may only be added to function declarations and change the code generated by the compiler when directly calling the function. The ``near`` attribute allows calls to the function to be made using the ``jal`` instruction, which requires the function to be defined in the same 256MB segment as the caller. The ``long_call`` and ``far`` attributes are synonyms and require the use of a different call sequence that works regardless of the distance between the functions. These attributes take priority over command line switches such as ``-mlong-calls``. ================ Comment at: lib/CodeGen/CGCall.cpp:1810 + FuncAttrs.addAttribute("near-call"); + } + ---------------- You should really put this in TargetCodeGenInfo::setTargetAttributes. Please just add a ForDefinition_t argument to that function and SetFunctionAttributes, then call setTargetAttributes from SetFunctionAttributes. ================ Comment at: lib/Sema/SemaDeclAttr.cpp:5955 + handleSimpleAttribute<MipsLongCallAttr>(S, D, Attr); + break; case AttributeList::AT_AMDGPUFlatWorkGroupSize: ---------------- You need to check for conflicts between the different attributes, and please add a test for that. Repository: rL LLVM https://reviews.llvm.org/D35479 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits