[clang] [llvm] [clang] Add support for the c2000 architecture (PR #125663)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Feb 5 02:05:52 PST 2025
student433 wrote:
> General comments:
>
> * Please do not use tabs for spacing. This is what causes the unexpected whitespace gaps in the diff.
>
> * There is a utility, clang-format-diff.py, which will assist in appeasing the code formatting check. However, I noticed running it myself that it changes some entire lists that should likely stay the way they are, so be careful!
>
> * My usual command line is something like: git diff -U0 --no-color --relative HEAD~1..HEAD | /path/to/clang-format-diff.py -p1 -i -binary /path/to/clang-format(.exe)
>
> * Is there no desire to catch and support C2000 built-in functions? Those will show up in clangd as undefined symbols and will be errors in c99 or later, and in all C++ modes.
>
> * Options
>
> * The options parser for clang and for the TI compilers are obviously very different. Some options interact with others for the purposes of ease-of-use and error checking, which is something that the clang options parser won't understand. I'll do my best to be diligent in case I have to point this out
> * There are many more options than the ones currently captured. Is it important we get as many as we can? Otherwise this will be a PR that is tailor-fit to a single use-case.
>
> * Is there a testing strategy you have in mind? At the very least, we need to cover the selection of the new mode and options. See clang/test/Driver/cl-options.c for an example
Thank you very much for your inputs,
I have yet to add the test cases for cl2000 driver in clang/tests/Driver and pair it with diagnostics as needed.
I will be sure to check the clang-format for indentation and other things. Will respond to each comment individually :)
> Excited to see your nice work!!!
>
> AFAIK all _TI extension keywords_ may confuse clang frontend, and treating them as macros would result in strange behaviors, e.g.
>
> ```c
> // DSP2833x_PieVect.h
> typedef interrupt void(*PINT)(void);
> ```
>
> So special keywords handling would be necessary...
>
> See [SPRU514Z - TMS320C28x Optimizing C/C++ Compiler](https://www.ti.com/lit/ug/spru514z/spru514z.pdf):
>
> > 6.5 Keywords
> > The C28x C/C++ compiler supports all of the standard C89 keywords, including const, volatile, and register. It supports all of the standard C99 keywords, including inline and restrict. It supports all of the standard C11 keywords. It also supports TI extension keywords `__interrupt`, `__cregister`, and `__asm`.
> > ...
> > 6.5.2 The `__cregister` Keyword
> > ...
> > 6.5.3 The `__interrupt` Keyword
> > ...
>
> CMIIW :) @DragonDisciple
Thank you very much for your inputs, I understand that these are semantically incorrect and would require a meaningful definition to avoid it being misunderstood. I could look into this :)
https://github.com/llvm/llvm-project/pull/125663
More information about the cfe-commits
mailing list