[PATCH] D46944: [analyzer] Use sufficiently large types for index/size calculation.

2018-05-24 Thread Bevin Hansson via Phabricator via cfe-commits
ebevhan added inline comments. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:89 SymMgr(context, BasicVals, alloc), MemMgr(context, alloc), -StateMgr(stateMgr), ArrayIndexTy(context.LongLongTy), +StateMgr(stateMgr),

[PATCH] D46986: [Fixed Point Arithmetic] Validation Test for Fixed Point Binary Operations and Saturated Addition

2018-05-23 Thread Bevin Hansson via Phabricator via cfe-commits
ebevhan added a comment. Also, this patch and all of the following 'Validation Test' patches do much more than just add tests, they add plenty of functionality as well. In general, it's quite difficult to tell which patches add what. I think it would be much clearer if the patches that claim

[PATCH] D46986: [Fixed Point Arithmetic] Validation Test for Fixed Point Binary Operations and Saturated Addition

2018-05-23 Thread Bevin Hansson via Phabricator via cfe-commits
ebevhan added a comment. I cannot say that I'm pleased with the CodeGen emission of the operations as pure IR. I can only assume that you do not have hardware specifically tailored for these operations, as matching this type of code ought to be quite difficult after optimization is performed.

[PATCH] D46927: [Fixed Point Arithmetic] Augmented Assignment for Fixed Point Types

2018-05-23 Thread Bevin Hansson via Phabricator via cfe-commits
ebevhan added inline comments. Comment at: include/clang/AST/Type.h:6577 + +inline unsigned getFixedPointFBits(const QualType ) { + return getFixedPointFBits(*Ty); As I mentioned in another patch, this should be in ASTContext and ask TargetInfo for the

[PATCH] D46917: [Fixed Point Arithmetic] Comparison and Unary Operations for Fixed Point Types

2018-05-23 Thread Bevin Hansson via Phabricator via cfe-commits
ebevhan added inline comments. Comment at: lib/CodeGen/CGExprScalar.cpp:2178 +llvm::Value *amt = +llvm::ConstantInt::get(value->getType(), 1 << fbits, + /*isSigned=*/type->isSignedFixedPointType()); ebevhan wrote: > Use

[PATCH] D46917: [Fixed Point Arithmetic] Comparison and Unary Operations for Fixed Point Types

2018-05-23 Thread Bevin Hansson via Phabricator via cfe-commits
ebevhan added inline comments. Comment at: lib/CodeGen/CGExprScalar.cpp:2141 + case BuiltinType::ShortAccum: +fbits = BUILTIN_SACCUM_FBIT; +break; Please see my comments on other patches about these values. Comment at:

[PATCH] D46915: [Fixed Point Arithmetic] Set Fixed Point Precision Bits and Create Fixed Point Literals

2018-05-23 Thread Bevin Hansson via Phabricator via cfe-commits
ebevhan added a comment. You should not define the fixed-point precision as compiler macros at build configure time. The number of fractional bits (the scaling factor) should be added to TargetInfo as target-configurable variables/accessors, and an accessor should be added to ASTContext (we

[PATCH] D46911: [Fixed Point Arithmetic] Addition of the remaining fixed point types and their saturated equivalents

2018-05-23 Thread Bevin Hansson via Phabricator via cfe-commits
ebevhan added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:206 +def err_invalid_saturation_spec : Error<"'%0' cannot be saturated. Only _Fract and _Accum can.">; def err_invalid_sign_spec : Error<"'%0' cannot be signed or unsigned">;

[PATCH] D46960: [Fixed Point Arithmetic] Predefined Precision Macros

2018-05-23 Thread Bevin Hansson via Phabricator via cfe-commits
ebevhan added inline comments. Comment at: include/clang/Lex/Preprocessor.h:185 + // Fractional bits of _Accum types + IdentifierInfo *Ident__SACCUM_FBIT__;// __SACCUM_FBIT__ + IdentifierInfo *Ident__ACCUM_FBIT__; // __ACCUM_FBIT__ You

[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type

2018-05-23 Thread Bevin Hansson via Phabricator via cfe-commits
ebevhan added inline comments. Comment at: lib/AST/ASTContext.cpp:1775 +case BuiltinType::UShortAccum: Width = Target->getShortWidth(); Align = Target->getShortAlign(); Please give the types their own width and alignment accessors/variables in

[PATCH] D46944: [analyzer] Use sufficiently large types for index/size calculation.

2018-05-21 Thread Bevin Hansson via Phabricator via cfe-commits
ebevhan updated this revision to Diff 147738. ebevhan edited the summary of this revision. ebevhan added a comment. Made ArrayIndexTy into ssize_t, consolidated the tests and fixed the test that was failing. https://reviews.llvm.org/D46944 Files:

[PATCH] D46944: [analyzer] Use sufficiently large types for index/size calculation.

2018-05-16 Thread Bevin Hansson via Phabricator via cfe-commits
ebevhan added inline comments. Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1344 // This is a signed value, since it's used in arithmetic with signed indices. - return svalBuilder.makeIntVal(RegionSize / EleSize, false); + return svalBuilder.makeIntVal(RegionSize /

[PATCH] D46944: [analyzer] Use sufficiently large types for index/size calculation.

2018-05-16 Thread Bevin Hansson via Phabricator via cfe-commits
ebevhan created this revision. ebevhan added reviewers: dergachev.a, dcoughlin. Herald added subscribers: cfe-commits, a.sidorin, szepet, xazax.hun. Herald added a reviewer: george.karpenkov. RegionStoreManager::getSizeInElements used 'int' for size calculations, and ProgramState::assumeInBound

[PATCH] D45865: [Sema] Emit -Warray-bounds for multiple levels of subscript expressions.

2018-04-24 Thread Bevin Hansson via Phabricator via cfe-commits
ebevhan added a comment. Thank you! I do not have commit rights, so if someone could commit this that would be great. https://reviews.llvm.org/D45865 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D45865: [Sema] Emit -Warray-bounds for multiple levels of subscript expressions.

2018-04-24 Thread Bevin Hansson via Phabricator via cfe-commits
ebevhan updated this revision to Diff 143684. ebevhan added a comment. Removed local variable and added some more to the test. https://reviews.llvm.org/D45865 Files: lib/Sema/SemaChecking.cpp test/SemaCXX/array-bounds.cpp test/SemaCXX/constant-expression-cxx11.cpp Index:

[PATCH] D45865: [Sema] Emit -Warray-bounds for multiple levels of subscript expressions.

2018-04-20 Thread Bevin Hansson via Phabricator via cfe-commits
ebevhan created this revision. ebevhan added reviewers: danielmarjamaki, aaron.ballman. Herald added a subscriber: cfe-commits. This patch has CheckArrayBounds recurse into ArraySubscriptExprs and MemberExprs, giving warnings for invalid indices for every level of subscript instead of just the

[PATCH] D37254: [Sema] Disallow assigning record lvalues with nested const-qualified fields.

2017-09-08 Thread Bevin Hansson via Phabricator via cfe-commits
bevinh updated this revision to Diff 114340. bevinh marked an inline comment as done. bevinh added a comment. Added a diag note for NestedConstMember. https://reviews.llvm.org/D37254 Files: include/clang/AST/Expr.h include/clang/AST/Type.h include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D37254: [Sema] Disallow assigning record lvalues with nested const-qualified fields.

2017-08-29 Thread Bevin Hansson via Phabricator via cfe-commits
bevinh created this revision. According to C99 6.3.2.1p1, structs and unions with nested const-qualified fields (that is, const-qualified fields declared at some recursive level of the aggregate) are not modifiable lvalues. However, Clang permits assignments of these lvalues. With this patch, we

<    1   2   3   4