aykevl accepted this revision.
aykevl added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: jacquesguan.
Looks good to me!
================
Comment at: clang/lib/Basic/Targets/AVR.cpp:27
const char *DefineName;
+ const int MaxFlashBank; // -1 means the device does not support LPM/ELPM.
};
----------------
This works and is fine, but I think you could also name it `NumFlashBanks` so
that it is the number of flash banks supported on the device (0 if LPM/ELPM is
not supported). With 0 for the attiny11 (which has no accessible flash banks)
and 1 for the attiny22 (because it has only one flash bank that can be
accessed: flash bank 0).
Just a suggestion, the current system looks good to me.
================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:8290
+ if (isTargetAddressSpace(AS) && 1 <= toTargetAddressSpace(AS) &&
+ toTargetAddressSpace(AS) <= 6 && !D->getType().isConstQualified())
CGM.getDiags().Report(D->getLocation(),
----------------
These hardcoded numbers are a bit unfortunate, but OK.
================
Comment at: clang/test/Sema/avr-flash.c:10
+ static __flash5 const int a5[] = {4, 6}; // expected-error {{unknown type
name '__flash5'}}
+ // TODO: It would be better to report "'__flash5' is not supported on
at908515".
+ return a0[n] + a1[n];
----------------
I think this can be implemented by always setting the `__flash*` defines and
checking whether they are valid for the current device in
`getGlobalVarAddressSpace`.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D115982/new/
https://reviews.llvm.org/D115982
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
