[PATCH] D134749: [clang][Interp] Implement Div opcode

2022-10-14 Thread Timm Bäder via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd704ba26b914: [clang][Interp] Implement div opcode (authored by tbaeder). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

[PATCH] D134749: [clang][Interp] Implement Div opcode

2022-10-06 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added a comment. In D134749#3839980 , @aaron.ballman wrote: > LGTM aside from a simple refactoring (feel free to apply it when landing or > do it post-commit as an NFC change). I actually suggested something like this review :P I'll write it do

[PATCH] D134749: [clang][Interp] Implement Div opcode

2022-10-06 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM aside from a simple refactoring (feel free to apply it when landing or do it post-commit as an NFC change). Comment at: clang/lib/AST/Interp/Interp.h:203-211 + if (LHS.isSigned() && LHS.isMin() && RHS.i

[PATCH] D134749: [clang][Interp] Implement Div opcode

2022-10-06 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder updated this revision to Diff 465667. tbaeder marked an inline comment as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134749/new/ https://reviews.llvm.org/D134749 Files: clang/lib/AST/Interp/ByteCodeExprGen.cpp clang/lib/AST/Interp/Integral.h clang/lib/AST/Interp/In

[PATCH] D134749: [clang][Interp] Implement Div opcode

2022-10-05 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added inline comments. Comment at: clang/test/AST/Interp/literals.cpp:171 + +namespace div { + constexpr int zero() { return 0; } Can we add a test case for `INT_MIN / -1` same for `%` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[PATCH] D134749: [clang][Interp] Implement Div opcode

2022-09-29 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder marked an inline comment as done. tbaeder added inline comments. Comment at: clang/test/AST/Interp/literals.cpp:181 + + + constexpr int LHS = 12; aaron.ballman wrote: > The only test coverage is for integer division; do you intend to also cover > floati

[PATCH] D134749: [clang][Interp] Implement Div opcode

2022-09-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/test/AST/Interp/literals.cpp:181 + + + constexpr int LHS = 12; The only test coverage is for integer division; do you intend to also cover floating-point division? Repository: rG LLVM Github Monorepo C

[PATCH] D134749: [clang][Interp] Implement Div opcode

2022-09-27 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added inline comments. Comment at: clang/lib/AST/Interp/Interp.h:183 + + if (RHS.isZero()) { +const SourceInfo &Loc = S.Current->getSource(OpPC); Just like rem we need to catch `INT_MIN / -1` `CheckICE(...)` looks like it does checking for both in th

[PATCH] D134749: [clang][Interp] Implement Div opcode

2022-09-27 Thread Erich Keane via Phabricator via cfe-commits
erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land. Seems good enoguh to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134749/new/ https://reviews.llvm.org/D134749 ___

[PATCH] D134749: [clang][Interp] Implement Div opcode

2022-09-27 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added inline comments. Comment at: clang/lib/AST/Interp/Interp.h:189 + const unsigned Bits = RHS.bitWidth() * 2; + T Result; + if (!T::div(LHS, RHS, Bits, &Result)) { Could use a helper function here and in `Rem` to reduce code duplication. Repositor

[PATCH] D134749: [clang][Interp] Implement Div opcode

2022-09-27 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder created this revision. tbaeder added reviewers: aaron.ballman, erichkeane, tahonermann, shafik. Herald added a project: All. tbaeder requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Same as the `rem` patch basically. Repository: