benshi001 marked an inline comment as done.
benshi001 added inline comments.


================
Comment at: clang/lib/Basic/Targets/AVR.cpp:313
+  Builder.defineMacro("__UINT16_TYPE__", "unsigned int");
+  Builder.defineMacro("__INT16_TYPE__", "int");
 
----------------
efriedma wrote:
> benshi001 wrote:
> > efriedma wrote:
> > > Redefining `__INT16_TYPE__` like this is unusual.  The macro is normally 
> > > defined in InitPreprocessor.cpp; if the logic there is wrong, I'd prefer 
> > > to fix it there.
> > > 
> > > For specific types int16_t and uint16_t, I'm not sure if anything inside 
> > > the compiler actually uses them at the moment, but I'd prefer to encode 
> > > the type in the TargetInfo, along the lines of getInt64Type(), in case we 
> > > need these types elsewhere in the future.
> > Making changes to InitPreprocessor.cpp will affect all targets, and I not 
> > familar with other targets. Actually AVR is so special that it defines  
> > `__INT16_TYPE__` to `int`.
> > Making changes to InitPreprocessor.cpp will affect all targets, and I not 
> > familar with other targets.
> 
> That shouldn't be a problem as long as you have an appropriate reviewer; I 
> volunteer. :)
> 
> You should be able to use getInt64Type() as a model for the necessary changes.
Thanks for your suggestion. I have submitted a patch,

https://reviews.llvm.org/D102547, and you are appreciated to review it. ^_^


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100701/new/

https://reviews.llvm.org/D100701

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to