rengolin added a comment.
Hi Alexandros,
My interpretation of `Tag_ABI_enum_size` is that value 3 is that **all** enums
are 32-bit and value 4 is that only those ABI-visible (ie. public interfaces)
have to be 32-bits. This is similar to the other PCS tags.
So, the table is:
- 0: no enums
- 1: short enums
- 2: all 32-bit
- 3: public 32-bit
With the default being 2 in GCC.
My reading of your code here is that you assume short|abi = short, which
doesn't match the table in the ABI.
================
Comment at: lib/Basic/Targets.cpp:5407
+ Builder.defineMacro("__ARM_SIZEOF_MINIMAL_ENUM",
+ Opts.ShortEnums || Opts.ABIEnums ? "1" : "4");
----------------
Isn't ABIEnums 4? Shouldn't this be:
Opts.ShortEnums || !Opts.ABIEnums
Is it even valid to have ABIEnums && ShortEnums at the same time?
================
Comment at: lib/Driver/Tools.cpp:5731
+
+ // The last of -fno-enums, -fshort-enums, -fabi-enums wins.
+ Arg *Enum;
----------------
If this is true, why go the extra complexity of mapping all possible states?
Why not just use one line:
Enum = Args.getLastArg(options::OPT_fno_enums, options::OPT_fshort_enums,
options::OPT_fabi_enums);
and be done with it? Short and ABI are not compatible, it's either one or the
other.
================
Comment at: lib/Driver/Tools.cpp:5741
+ else
+ Enum = Args.getLastArg(options::OPT_fno_enums);
+ if (Enum) {
----------------
This code is confusing and merging with each other. Please add some empty lines
between them.
https://reviews.llvm.org/D26968
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
