erichkeane accepted this revision.
erichkeane added inline comments.
This revision is now accepted and ready to land.
================
Comment at: clang/lib/AST/Interp/Integral.h:213
+ static bool rem(Integral A, Integral B, unsigned OpBits, Integral *R) {
+ *R = Integral(A.V % B.V);
----------------
tbaeder wrote:
> erichkeane wrote:
> > Its a touch weird we return 'bool' in all these places, rather than an
> > `Optional<Integral>`, right? Perhaps a refactor idea for a future patch.
> The really weird part is that `false` here suddenly means success, but I get
> your point. Does using `Optional` introduce any sort of performance penalty
> if this is being called a lot?
'false' as success is actually pretty common in clang, we use that all over the
place. I don't believe Optional would have any negative penalty, at least
compared to a pointer-param?
================
Comment at: clang/lib/AST/Interp/Opcodes.td:57
-def AluTypeClass : TypeClass {
+def NumberTypeClass : TypeClass {
let Types = [Sint8, Uint8, Sint16, Uint16, Sint32,
----------------
tbaeder wrote:
> erichkeane wrote:
> > Perhaps `IntegralTypeClass`, unless we expect floats to be here too?
> Floats don't have to be in there I guess, but so far I'd say yes, they should
> be, because they are usable everywhere this class is used, afaik.
Ok then, thanks.
================
Comment at: clang/lib/AST/Interp/Opcodes.td:62
+
+def AluTypeClass : TypeClass {
+ let Types = !listconcat(NumberTypeClass.Types, [Bool]);
----------------
tbaeder wrote:
> erichkeane wrote:
> > What does "Alu" mean here?
> I didn't introduce the name but I assumed it comes from "arithmetic logical
> unit".
Ah! That could be. Makes sense.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134744/new/
https://reviews.llvm.org/D134744
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
