Re: [PATCH] D24311: Implement MS _rot intrinsics

2016-09-08 Thread Albert Gutowski via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL280997: Implement MS _rot intrinsics (authored by agutowski). Changed prior to commit: https://reviews.llvm.org/D24311?vs=70747&id=70759#toc Repository: rL LLVM https://reviews.llvm.org/D24311 Files

Re: [PATCH] D24311: Implement MS _rot intrinsics

2016-09-08 Thread Reid Kleckner via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm https://reviews.llvm.org/D24311 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24311: Implement MS _rot intrinsics

2016-09-08 Thread Albert Gutowski via cfe-commits
agutowski marked 4 inline comments as done. agutowski added a comment. https://reviews.llvm.org/D24311 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24311: Implement MS _rot intrinsics

2016-09-08 Thread Albert Gutowski via cfe-commits
agutowski updated this revision to Diff 70747. agutowski added a comment. Remove evaluating values for constant arguments https://reviews.llvm.org/D24311 Files: include/clang/Basic/Builtins.def lib/CodeGen/CGBuiltin.cpp lib/Headers/intrin.h test/CodeGen/ms-intrinsics-rotations.c Index:

Re: [PATCH] D24311: Implement MS _rot intrinsics

2016-09-08 Thread Reid Kleckner via cfe-commits
rnk added inline comments. Comment at: lib/AST/ExprConstant.cpp:7024-7050 @@ -7023,1 +7023,29 @@ + case Builtin::BI_rotl8: + case Builtin::BI_rotl16: + case Builtin::BI_rotl: + case Builtin::BI_lrotl: + case Builtin::BI_rotl64: { +APSInt Val, Shift; +if (!EvaluateIn

Re: [PATCH] D24311: Implement MS _rot intrinsics

2016-09-08 Thread David Majnemer via cfe-commits
majnemer added inline comments. Comment at: lib/AST/ExprConstant.cpp:7024-7050 @@ -7023,1 +7023,29 @@ + case Builtin::BI_rotl8: + case Builtin::BI_rotl16: + case Builtin::BI_rotl: + case Builtin::BI_lrotl: + case Builtin::BI_rotl64: { +APSInt Val, Shift; +if (!Evalu

Re: [PATCH] D24311: Implement MS _rot intrinsics

2016-09-08 Thread Albert Gutowski via cfe-commits
agutowski added inline comments. Comment at: lib/AST/ExprConstant.cpp:7024-7050 @@ -7023,1 +7023,29 @@ + case Builtin::BI_rotl8: + case Builtin::BI_rotl16: + case Builtin::BI_rotl: + case Builtin::BI_lrotl: + case Builtin::BI_rotl64: { +APSInt Val, Shift; +if (!Eval

Re: [PATCH] D24311: Implement MS _rot intrinsics

2016-09-07 Thread David Majnemer via cfe-commits
majnemer added inline comments. Comment at: lib/AST/ExprConstant.cpp:7024-7050 @@ -7023,1 +7023,29 @@ + case Builtin::BI_rotl8: + case Builtin::BI_rotl16: + case Builtin::BI_rotl: + case Builtin::BI_lrotl: + case Builtin::BI_rotl64: { +APSInt Val, Shift; +if (!Evalu

Re: [PATCH] D24311: Implement MS _rot intrinsics

2016-09-07 Thread Albert Gutowski via cfe-commits
agutowski updated this revision to Diff 70646. agutowski marked an inline comment as done. agutowski added a comment. Add evaluating values for constant arguments https://reviews.llvm.org/D24311 Files: include/clang/Basic/Builtins.def lib/AST/ExprConstant.cpp lib/CodeGen/CGBuiltin.cpp l

Re: [PATCH] D24311: Implement MS _rot intrinsics

2016-09-07 Thread David Majnemer via cfe-commits
majnemer added a subscriber: majnemer. majnemer added a comment. In https://reviews.llvm.org/D24311#536545, @agutowski wrote: > In https://reviews.llvm.org/D24311#536333, @rnk wrote: > > > You should locally verify that this generates the correct assembly when > > optimizations are enabled, and

Re: [PATCH] D24311: Implement MS _rot intrinsics

2016-09-07 Thread Albert Gutowski via cfe-commits
agutowski marked 2 inline comments as done. agutowski added a comment. In https://reviews.llvm.org/D24311#536333, @rnk wrote: > You should locally verify that this generates the correct assembly when > optimizations are enabled, and if it doesn't, try to make the input look more > like llvm/tes

Re: [PATCH] D24311: Implement MS _rot intrinsics

2016-09-07 Thread Albert Gutowski via cfe-commits
agutowski updated this revision to Diff 70614. agutowski added a comment. Fix undefined value problem when rotating by zero bits Change tests to work without -Oz flag Fix types of arguments in tests https://reviews.llvm.org/D24311 Files: include/clang/Basic/Builtins.def lib/CodeGen/CGBuilti

Re: [PATCH] D24311: Implement MS _rot intrinsics

2016-09-07 Thread Reid Kleckner via cfe-commits
rnk added a comment. You should locally verify that this generates the correct assembly when optimizations are enabled, and if it doesn't, try to make the input look more like llvm/test/CodeGen/X86/rotate.ll Comment at: test/CodeGen/ms-intrinsics-rotations.c:2 @@ +1,3 @@ +// R

Re: [PATCH] D24311: Implement MS _rot intrinsics

2016-09-07 Thread Reid Kleckner via cfe-commits
rnk added inline comments. Comment at: lib/CodeGen/CGBuiltin.cpp:740 @@ +739,3 @@ +Value *LeftShifted = Builder.CreateShl(Val, Shift); +Value *RightShifted = Builder.CreateLShr(Val, RightShift); +Value *Shifted = Builder.CreateOr(LeftShifted, RightShifted); ---

[PATCH] D24311: Implement MS _rot intrinsics

2016-09-07 Thread Albert Gutowski via cfe-commits
agutowski created this revision. agutowski added reviewers: rnk, thakis, Prazek, compnerd. agutowski added a subscriber: cfe-commits. https://reviews.llvm.org/D24311 Files: include/clang/Basic/Builtins.def lib/CodeGen/CGBuiltin.cpp lib/Headers/intrin.h test/CodeGen/ms-intrinsics-rotations