Re: [PATCH] D12797: Refactor LoopConvertCheck.

[Manuel Klimek via cfe-commits]

klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.

lg


http://reviews.llvm.org/D12797



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12903: Add -fplugin=name.so option to the driver

[Nico Weber via cfe-commits]

thakis added a subscriber: thakis.
thakis added a comment.

Is the idea that this loads plugins that link against clang's C++ api? That api 
isn't considered stable, so not having an "official" flag for this always 
looked by design to me. Also, C++ doesn't make for a good ABI. Also also, this 
approach fundamentally doesn't work on Windows.


Repository:
  rL LLVM

http://reviews.llvm.org/D12903



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D12996: Driver: support ARM/HF on a single toolchain

[Saleem Abdulrasool via cfe-commits]

compnerd created this revision.
compnerd added a reviewer: rengolin.
compnerd added a subscriber: cfe-commits.
Herald added subscribers: srhines, danalbert, tberghammer, rengolin, aemerson.

ARM EABI adds target attributes to the object file.  Amongst the attributes that
are emitted is the VFP argument passing (Hard vs Soft).  The linker is
responsible for checking these attributes and erroring on mismatches.  This
causes problems for the compiler-rt builtins when targeting both hard and
soft.  Because both of these options name the builtins compiler-rt component
the same (libclang_rt.builtins-arm.a or libclang_rt.builtins-arm-android).  GCC
is able to get away with this as it does one target per toolchain.  This
changes the naming convention for the ARM compiler-rt builtins to differentiate
between HF and Soft.  Although this means that compiler-rt may be duplicated, it
enables supporting both variants from a single toolchain.  A similar approach is
taken by the Darwin toolchain, naming the library to differentiate between the
calling conventions.

http://reviews.llvm.org/D12996

Files:
  lib/Driver/SanitizerArgs.cpp
  lib/Driver/Tools.cpp
  lib/Driver/Tools.h
  test/Driver/arm-compiler-rt.c

Index: test/Driver/arm-compiler-rt.c
===
--- /dev/null
+++ test/Driver/arm-compiler-rt.c
@@ -0,0 +1,21 @@
+// RUN: %clang -target arm-linux-gnueabi -rtlib=compiler-rt -### %s 2>&1 | FileCheck %s -check-prefix ARM-GNUEABI
+// ARM-GNUEABI: "{{.*}}/libclang_rt.builtins-arm.a"
+
+// RUN: %clang -target arm-linux-gnueabi -rtlib=compiler-rt -mfloat-abi=hard -### %s 2>&1 | FileCheck %s -check-prefix ARM-GNUEABI-ABI
+// ARM-GNUEABI-ABI: "{{.*}}/libclang_rt.builtins-armhf.a"
+
+// RUN: %clang -target arm-linux-gnueabihf -rtlib=compiler-rt -### %s 2>&1 | FileCheck %s -check-prefix ARM-GNUEABIHF
+// ARM-GNUEABIHF: "{{.*}}/libclang_rt.builtins-armhf.a"
+
+// RUN: %clang -target arm-linux-gnueabihf -rtlib=compiler-rt -mfloat-abi=soft -### %s 2>&1 | FileCheck %s -check-prefix ARM-GNUEABIHF-ABI
+// ARM-GNUEABIHF-ABI: "{{.*}}/libclang_rt.builtins-arm.a"
+
+// RUN: %clang -target arm-windows-itanium -rtlib=compiler-rt -### %s 2>&1 | FileCheck %s -check-prefix ARM-WINDOWS
+// ARM-WINDOWS: "{{.*}}/clang_rt.builtins-arm.lib"
+
+// RUN: %clang -target arm-linux-androideabi -rtlib=compiler-rt -### %s 2>&1 | FileCheck %s -check-prefix ARM-ANDROID
+// ARM-ANDROID: "{{.*}}/libclang_rt.builtins-arm-android.a"
+
+// RUN: %clang -target arm-linux-androideabi -rtlib=compiler-rt -mfloat-abi=hard -### %s 2>&1 | FileCheck %s -check-prefix ARM-ANDROIDHF
+// ARM-ANDROIDHF: "{{.*}}/libclang_rt.builtins-armhf-android.a"
+
Index: lib/Driver/Tools.h
===
--- lib/Driver/Tools.h
+++ lib/Driver/Tools.h
@@ -37,8 +37,9 @@
 
 using llvm::opt::ArgStringList;
 
-SmallString<128> getCompilerRT(const ToolChain , StringRef Component,
-   bool Shared = false);
+SmallString<128> getCompilerRT(const ToolChain ,
+   const llvm::opt::ArgList ,
+   StringRef Component, bool Shared = false);
 
 /// \brief Clang compiler tool.
 class LLVM_LIBRARY_VISIBILITY Clang : public Tool {
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -2398,12 +2398,19 @@
 }
 
 // Until ARM libraries are build separately, we have them all in one library
-static StringRef getArchNameForCompilerRTLib(const ToolChain ) {
-  if (TC.getTriple().isWindowsMSVCEnvironment() &&
-  TC.getArch() == llvm::Triple::x86)
+static StringRef getArchNameForCompilerRTLib(const ToolChain ,
+ const ArgList ) {
+  const llvm::Triple  = TC.getTriple();
+  bool IsWindows = Triple.isOSWindows();
+
+  if (Triple.isWindowsMSVCEnvironment() && TC.getArch() == llvm::Triple::x86)
 return "i386";
+
   if (TC.getArch() == llvm::Triple::arm || TC.getArch() == llvm::Triple::armeb)
-return "arm";
+return (arm::getARMFloatABI(TC, Args) == arm::FloatABI::Hard && !IsWindows)
+   ? "armhf"
+   : "arm";
+
   return TC.getArchName();
 }
 
@@ -2418,16 +2425,16 @@
   return Res;
 }
 
-SmallString<128> tools::getCompilerRT(const ToolChain , StringRef Component,
-  bool Shared) {
+SmallString<128> tools::getCompilerRT(const ToolChain , const ArgList ,
+  StringRef Component, bool Shared) {
   const char *Env = TC.getTriple().getEnvironment() == llvm::Triple::Android
 ? "-android"
 : "";
 
   bool IsOSWindows = TC.getTriple().isOSWindows();
   bool IsITANMSVCWindows = TC.getTriple().isWindowsMSVCEnvironment() ||
TC.getTriple().isWindowsItaniumEnvironment();
-  StringRef Arch = getArchNameForCompilerRTLib(TC);

r248094 - Driver: tweak ARM target feature calculation

[Saleem Abdulrasool via cfe-commits]

Author: compnerd
Date: Sat Sep 19 13:19:44 2015
New Revision: 248094

URL: http://llvm.org/viewvc/llvm-project?rev=248094=rev
Log:
Driver: tweak ARM target feature calculation

Rather than using re-calculating the effective triple, thread the already
calculated value down into AddARMTargetArgs.  This avoids both recreating the
triple, as well as re-parsing the triple as it was already done in the previous
frame.

Modified:
cfe/trunk/lib/Driver/Tools.cpp
cfe/trunk/lib/Driver/Tools.h

Modified: cfe/trunk/lib/Driver/Tools.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=248094=248093=248094=diff
==
--- cfe/trunk/lib/Driver/Tools.cpp (original)
+++ cfe/trunk/lib/Driver/Tools.cpp Sat Sep 19 13:19:44 2015
@@ -851,15 +851,9 @@ static void getARMTargetFeatures(const D
 Features.push_back("+no-movt");
 }
 
-void Clang::AddARMTargetArgs(const ArgList , ArgStringList ,
- bool KernelOrKext) const {
-  const Driver  = getToolChain().getDriver();
-  // Get the effective triple, which takes into account the deployment target.
-  std::string TripleStr = getToolChain().ComputeEffectiveClangTriple(Args);
-  llvm::Triple Triple(TripleStr);
-
+void Clang::AddARMTargetArgs(const llvm::Triple , const ArgList ,
+ ArgStringList , bool KernelOrKext) const {
   // Select the ABI to use.
-  //
   // FIXME: Support -meabi.
   // FIXME: Parts of this are duplicated in the backend, unify this somehow.
   const char *ABIName = nullptr;
@@ -898,7 +892,8 @@ void Clang::AddARMTargetArgs(const ArgLi
   CmdArgs.push_back(ABIName);
 
   // Determine floating point ABI from the options & target defaults.
-  arm::FloatABI ABI = arm::getARMFloatABI(D, Args, Triple);
+  arm::FloatABI ABI =
+  arm::getARMFloatABI(getToolChain().getDriver(), Args, Triple);
   if (ABI == arm::FloatABI::Soft) {
 // Floating point operations and argument passing are soft.
 // FIXME: This changes CPP defines, we need -target-soft-float.
@@ -3676,7 +3671,8 @@ void Clang::ConstructJob(Compilation ,
   case llvm::Triple::armeb:
   case llvm::Triple::thumb:
   case llvm::Triple::thumbeb:
-AddARMTargetArgs(Args, CmdArgs, KernelOrKext);
+// Use the effective triple, which takes into account the deployment 
target.
+AddARMTargetArgs(Triple, Args, CmdArgs, KernelOrKext);
 break;
 
   case llvm::Triple::aarch64:

Modified: cfe/trunk/lib/Driver/Tools.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.h?rev=248094=248093=248094=diff
==
--- cfe/trunk/lib/Driver/Tools.h (original)
+++ cfe/trunk/lib/Driver/Tools.h Sat Sep 19 13:19:44 2015
@@ -59,7 +59,8 @@ private:
 
   void AddAArch64TargetArgs(const llvm::opt::ArgList ,
 llvm::opt::ArgStringList ) const;
-  void AddARMTargetArgs(const llvm::opt::ArgList ,
+  void AddARMTargetArgs(const llvm::Triple ,
+const llvm::opt::ArgList ,
 llvm::opt::ArgStringList ,
 bool KernelOrKext) const;
   void AddARM64TargetArgs(const llvm::opt::ArgList ,


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r247618 - C11 _Bool bitfield diagnostic

[Nico Weber via cfe-commits]

On Fri, Sep 18, 2015 at 10:19 PM, Richard Smith 
wrote:

> On Fri, Sep 18, 2015 at 8:49 PM, Nico Weber  wrote:
>
>> On Fri, Sep 18, 2015 at 5:06 PM, Richard Smith 
>> wrote:
>>
>>> On Wed, Sep 16, 2015 at 5:33 PM, Richard Smith 
>>> wrote:
>>>
 On Wed, Sep 16, 2015 at 5:27 PM, Nico Weber via cfe-commits <
 cfe-commits@lists.llvm.org> wrote:

> On Tue, Sep 15, 2015 at 5:50 PM, Richard Smith 
> wrote:
>
>> On Tue, Sep 15, 2015 at 12:38 PM, Nico Weber 
>> wrote:
>>
>>> With this patch, we warn on `bool a : 4;`, yet we don't warn on
>>> `bool b` (which has 8 bits storage, 1 bit value). Warning on `bool b` is
>>> silly of course, but why is warning on `bool a : 4` useful? That's like 
>>> 50%
>>> more storage efficient than `bool b` ;-)
>>>
>>> It's possible that this is a good warning for some reason, but I
>>> don't quite see why yet.
>>>
>>
>> Why would we warn on "unsigned n : 57;"? The bit-field is wider than
>> necessary, and we have no idea what the programmer was trying to do
>>
>
> Warning on this kind of makes sense to me, as the field is wider than
> the default width of int. (Not warning on that doesn't seem terrible to me
> either though.)
>
> I'm only confused about the bool case with bitfield sizes < 8 I think.
> We warn that the bitfield is wider than the value size, even though it's
> smaller than the default storage size, and we don't warn on regular bools.
>
> To get an idea how often this warning fires, I ran it on a large-ish
> open source codebase I had flying around. The only place it fired on is 
> one
> header in protobuf (extension_set.h). I looked at the history of that 
> file,
> and it had a struct that used to look like
>
>   struct Extension {
> SomeEnum e;
> bool a;
> bool b;
> bool c;
> int d;
> // ...some more stuff...
>   };
>
> Someone then added another field to this and for some reason decided
> to do it like so:
>
>   struct Extension {
> SomeEnum e;
> bool a;
> bool b1 : 4;
> bool b2 : 4;
> bool c;
> int d;
> // ...some more stuff...
>   };
>
> Neither the commit message nor the review discussion mention the
> bitfield at all as far as I can tell. Now, given that this isn't a small
> struct and it has a bunch of normal bools, I don't know why they added the
> new field as bitfield while this wasn't deemed necessary for the existing
> bools. My best guess is that that they didn't want to add 3 bytes of
> padding (due to the int field), which seems like a decent reason.
>
> Had the warning been in place when this code got written, I suppose
> they had used ": 1" instead. Does this make this code much better? It
> doesn't seem like it to me. So after doing a warning quality eval, I'd
> suggest to not emit the warning for bool bitfields if the bitfield size is
> < 8. (But since the warning fires only very rarely, I don't feel very
> strongly about this.)
>

 I agree it doesn't make the code /much/ better. But if I were reading
 that, I would certainly pause for a few moments wondering what the author
 was thinking. I also don't feel especially strongly about this, but I don't
 see a good rationale for warning on 'bool : 9' but not on 'bool : 5'.

>>>
>>> I'm coming around to the opinion that we shouldn't give this warning on
>>> bool at all -- the point of the warning is to point out that an 'unsigned :
>>> 40;' bitfield can't hold 2**40 - 1, and values of that size will be
>>> truncated. There is no corresponding problematic case for bool, so we have
>>> a much weaker justification for warning in this case -- we have no idea
>>> what the user was trying to achieve, but we do not have a signal that their
>>> code is wrong.
>>>
>>> Thoughts?
>>>
>>
>> Makes sense to me :-) What about `bool : 16`?
>>
>
> I don't think it makes sense to treat bool : 3 and bool : 16 differently.
> The fact that an unadorned bool would occupy 8 bits doesn't seem relevant
> to whether we should warn. Either we warn that there are padding bits, or
> we don't.
>

Yup, makes sense.


>
>
>> , but it doesn't seem likely they got that effect. Would you be more
>> convinced if we amended the diagnostic to provide a fixit suggesting 
>> using
>> an anonymous bit-field to insert padding?
>>
>
> Isn't the Right Fix (tm) to make bool bitfields 1 wide and rely on the
> compiler to figure out padding?
>

 It depends; maybe the intent is to be compatible with some on-disk
 format, and the explicit padding is important:

 struct X {
   int n : 3;
   bool b 

Re: r245459 - According to i686 ABI, long double size on x86 is 12 bytes not 16 bytes.

[Yaron Keren via cfe-commits]

Thanks, I have replied there.

2015-09-19 13:33 GMT+03:00 Hal Finkel :

> FYI: https://llvm.org/bugs/show_bug.cgi?id=24398 was just reopened
> pointing to a lack of resolution here.
>
>  -Hal
>
> - Original Message -
> > From: "Yaron Keren via cfe-commits" 
> > To: "Martell Malone" 
> > Cc: "Richard Smith" , "cfe-commits" <
> cfe-commits@lists.llvm.org>
> > Sent: Friday, August 21, 2015 2:47:50 AM
> > Subject: Re: r245459 - According to i686 ABI, long double size on x86 is
> 12 bytes not 16 bytes.
> >
> >
> >
> >
> > The testcase from r245459 was not reverted and still in SVN.
> >
> >
> >
> >
> >
> > 2015-08-21 2:05 GMT+03:00 Martell Malone < martellmal...@gmail.com >
> > :
> >
> >
> >
> >
> >
> > I feel very silly now.
> > After testing the testcase again on svn it still works.
> > It appears the OP was looking for this patch to go onto the 3.6
> > branch and was applying my patch to that.
> >
> >
> > I'll know in future to recheck the testcase afterwards myself in
> > future.
> >
> >
> > Apologies for the noise guys.
> >
> >
> > Yaron I think the test case from r245459 would be useful to ensure it
> > is never broken in the future?
> >
> > Would you be able to recommit the test case?
> >
> >
> >
> > Kind Regards
> >
> > Martell
> >
> >
> >
> >
> >
> >
> >
> > On Thu, Aug 20, 2015 at 3:57 PM, Martell Malone <
> > martellmal...@gmail.com > wrote:
> >
> >
> >
> >
> >
> > There is no testcase for PR24398 nor the OP reporting the problem was
> > actually solved. Martell?
> > I'm just re-looking through it now.
> >
> >
> > X86TargetInfo sets LongDoubleFormat =
> > ::APFloat::x87DoubleExtended;
> > X86_64TargetInfo then sets LongDoubleWidth = LongDoubleAlign = 128 ;
> > X86_32 TargetInfo then sets LongDoubleWidth = 96 ; LongDoubleAlign =
> > 32 ;
> >
> >
> > From this I can see that the patch I committed actually doesn't
> > change anything but only breaks mingw x86.
> >
> >
> > I can only see these values changed in Microsoft*TargetInfo classes
> > which is not a parent of MINGW
> >
> >
> > When I submitted the patch I just wanted to explicitly set the values
> > in MinGWX86_64TargetInfo to ensure it was in fact that.
> >
> > It seemed as though it was not inheriting the value from the root
> > parent class somehow.
> >
> >
> >
> > I was told on irc that it did fix the bug which is even stranger.
> > I'm actually at a bit of a loss as to what the proper fix to this is
> > then.
> >
> > Apologies for breaking mingw i686 long double.
> >
> >
> > I will do up a test case and try to find the actual cause of the long
> > double bug and reopen the issue.
> >
> >
> >
> >
> >
> > On Thu, Aug 20, 2015 at 3:09 PM, Yaron Keren < yaron.ke...@gmail.com
> > > wrote:
> >
> >
> >
> >
> > Hi, I've just done this exactly this in r245618 (32 bit) and r245620
> > (64 bits).
> >
> >
> > mingw i686 long double values were correct before r245084 and wrong
> > after it.
> > mingw x86_64 long double values were not modified at all by r245084
> > for the reason you stated, so I agree and do not see how this
> > non-change can solve anything . There is no testcase for PR24398 nor
> > the OP reporting the problem was actually solved. Martell?
> >
> >
> > About PR24398, long double support was in clang long ago and b oth
> > code examples compile and run correctly with current svn (without
> > r245084) and gcc version 5.1.0 (i686-posix-dwarf-rev0, Built by
> > MinGW-W64 project).
> > It's not x86_64 but as said r245084 didn't actually modify x86_64
> > configuration.
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > 2015-08-21 0:52 GMT+03:00 Richard Smith < rich...@metafoo.co.uk > :
> >
> >
> > OK, so here's the problem:
> >
> >
> > The right way to fix this seems to be to delete the assignments to
> > LongDouble* from the MinGWX86_32TargetInfo constructor; the
> > X86_32TargetInfo and X86TargetInfo base classes already set them to
> > the right values. Likewise we can delete the assignments to
> > LongDouble* from the MinGWX86_64TargetInfo constructor for the same
> > reason.
> >
> >
> > But... that completely reverts Martell's r245084, which apparently
> > fixed PR24398. So you two need to figure out what the actual problem
> > is here, and what the right fix is. r245084 didn't provide any test
> > cases, and had no apparent effect other than to break long double
> > for mingw32; did it really fix PR24398 (and if so, how)?
> >
> >
> >
> >
> >
> > On Thu, Aug 20, 2015 at 2:27 PM, Yaron Keren < yaron.ke...@gmail.com
> > > wrote:
> >
> >
> >
> >
> > OK, based on testing, mingw i686 aligns long doubles to 4 bytes:
> >
> >
> >
> >
> > sh-4.3$ cat < a.cpp
> > #include 
> > int main() {
> > struct {
> > char c[1];
> > long double d;
> > } s;
> > std::coutstd::endl;
> > std::coutstd::endl;
> > }
> > sh-4.3$ g++ a.cpp&&./a.exe
> > 0x28fea0
> > 0x28fea4
> >
> >
> > I'll fix that.
> >
> >
> >
> >
> >
> >
> >
> > 2015-08-21 0:13 GMT+03:00 

r245459 - According to i686 ABI, long double size on x86 is 12 bytes not 16 bytes.

[Martell Malone via cfe-commits]

For reference It was raised again here also
https://github.com/Alexpux/MINGW-packages/issues/722


"that is extended from X86TargetInfo
Which sets LongDoubleFormat = ::APFloat::x87DoubleExtended;"

On Saturday, September 19, 2015, Yaron Keren > wrote:

> Thanks, I have replied there.
>
> 2015-09-19 13:33 GMT+03:00 Hal Finkel :
>
>> FYI: https://llvm.org/bugs/show_bug.cgi?id=24398 was just reopened
>> pointing to a lack of resolution here.
>>
>>  -Hal
>>
>> - Original Message -
>> > From: "Yaron Keren via cfe-commits" 
>> > To: "Martell Malone" 
>> > Cc: "Richard Smith" , "cfe-commits" <
>> cfe-commits@lists.llvm.org>
>> > Sent: Friday, August 21, 2015 2:47:50 AM
>> > Subject: Re: r245459 - According to i686 ABI, long double size on x86
>> is 12 bytes not 16 bytes.
>> >
>> >
>> >
>> >
>> > The testcase from r245459 was not reverted and still in SVN.
>> >
>> >
>> >
>> >
>> >
>> > 2015-08-21 2:05 GMT+03:00 Martell Malone < martellmal...@gmail.com >
>> > :
>> >
>> >
>> >
>> >
>> >
>> > I feel very silly now.
>> > After testing the testcase again on svn it still works.
>> > It appears the OP was looking for this patch to go onto the 3.6
>> > branch and was applying my patch to that.
>> >
>> >
>> > I'll know in future to recheck the testcase afterwards myself in
>> > future.
>> >
>> >
>> > Apologies for the noise guys.
>> >
>> >
>> > Yaron I think the test case from r245459 would be useful to ensure it
>> > is never broken in the future?
>> >
>> > Would you be able to recommit the test case?
>> >
>> >
>> >
>> > Kind Regards
>> >
>> > Martell
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> > On Thu, Aug 20, 2015 at 3:57 PM, Martell Malone <
>> > martellmal...@gmail.com > wrote:
>> >
>> >
>> >
>> >
>> >
>> > There is no testcase for PR24398 nor the OP reporting the problem was
>> > actually solved. Martell?
>> > I'm just re-looking through it now.
>> >
>> >
>> > X86TargetInfo sets LongDoubleFormat =
>> > ::APFloat::x87DoubleExtended;
>> > X86_64TargetInfo then sets LongDoubleWidth = LongDoubleAlign = 128 ;
>> > X86_32 TargetInfo then sets LongDoubleWidth = 96 ; LongDoubleAlign =
>> > 32 ;
>> >
>> >
>> > From this I can see that the patch I committed actually doesn't
>> > change anything but only breaks mingw x86.
>> >
>> >
>> > I can only see these values changed in Microsoft*TargetInfo classes
>> > which is not a parent of MINGW
>> >
>> >
>> > When I submitted the patch I just wanted to explicitly set the values
>> > in MinGWX86_64TargetInfo to ensure it was in fact that.
>> >
>> > It seemed as though it was not inheriting the value from the root
>> > parent class somehow.
>> >
>> >
>> >
>> > I was told on irc that it did fix the bug which is even stranger.
>> > I'm actually at a bit of a loss as to what the proper fix to this is
>> > then.
>> >
>> > Apologies for breaking mingw i686 long double.
>> >
>> >
>> > I will do up a test case and try to find the actual cause of the long
>> > double bug and reopen the issue.
>> >
>> >
>> >
>> >
>> >
>> > On Thu, Aug 20, 2015 at 3:09 PM, Yaron Keren < yaron.ke...@gmail.com
>> > > wrote:
>> >
>> >
>> >
>> >
>> > Hi, I've just done this exactly this in r245618 (32 bit) and r245620
>> > (64 bits).
>> >
>> >
>> > mingw i686 long double values were correct before r245084 and wrong
>> > after it.
>> > mingw x86_64 long double values were not modified at all by r245084
>> > for the reason you stated, so I agree and do not see how this
>> > non-change can solve anything . There is no testcase for PR24398 nor
>> > the OP reporting the problem was actually solved. Martell?
>> >
>> >
>> > About PR24398, long double support was in clang long ago and b oth
>> > code examples compile and run correctly with current svn (without
>> > r245084) and gcc version 5.1.0 (i686-posix-dwarf-rev0, Built by
>> > MinGW-W64 project).
>> > It's not x86_64 but as said r245084 didn't actually modify x86_64
>> > configuration.
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> > 2015-08-21 0:52 GMT+03:00 Richard Smith < rich...@metafoo.co.uk > :
>> >
>> >
>> > OK, so here's the problem:
>> >
>> >
>> > The right way to fix this seems to be to delete the assignments to
>> > LongDouble* from the MinGWX86_32TargetInfo constructor; the
>> > X86_32TargetInfo and X86TargetInfo base classes already set them to
>> > the right values. Likewise we can delete the assignments to
>> > LongDouble* from the MinGWX86_64TargetInfo constructor for the same
>> > reason.
>> >
>> >
>> > But... that completely reverts Martell's r245084, which apparently
>> > fixed PR24398. So you two need to figure out what the actual problem
>> > is here, and what the right fix is. r245084 didn't provide any test
>> > cases, and had no apparent effect other than to break long double
>> > for 

Re: r245459 - According to i686 ABI, long double size on x86 is 12 bytes not 16 bytes.

[Hal Finkel via cfe-commits]

FYI: https://llvm.org/bugs/show_bug.cgi?id=24398 was just reopened pointing to 
a lack of resolution here.

 -Hal

- Original Message -
> From: "Yaron Keren via cfe-commits" 
> To: "Martell Malone" 
> Cc: "Richard Smith" , "cfe-commits" 
> 
> Sent: Friday, August 21, 2015 2:47:50 AM
> Subject: Re: r245459 - According to i686 ABI, long double size on x86 is 12 
> bytes not 16 bytes.
> 
> 
> 
> 
> The testcase from r245459 was not reverted and still in SVN.
> 
> 
> 
> 
> 
> 2015-08-21 2:05 GMT+03:00 Martell Malone < martellmal...@gmail.com >
> :
> 
> 
> 
> 
> 
> I feel very silly now.
> After testing the testcase again on svn it still works.
> It appears the OP was looking for this patch to go onto the 3.6
> branch and was applying my patch to that.
> 
> 
> I'll know in future to recheck the testcase afterwards myself in
> future.
> 
> 
> Apologies for the noise guys.
> 
> 
> Yaron I think the test case from r245459 would be useful to ensure it
> is never broken in the future?
> 
> Would you be able to recommit the test case?
> 
> 
> 
> Kind Regards
> 
> Martell
> 
> 
> 
> 
> 
> 
> 
> On Thu, Aug 20, 2015 at 3:57 PM, Martell Malone <
> martellmal...@gmail.com > wrote:
> 
> 
> 
> 
> 
> There is no testcase for PR24398 nor the OP reporting the problem was
> actually solved. Martell?
> I'm just re-looking through it now.
> 
> 
> X86TargetInfo sets LongDoubleFormat =
> ::APFloat::x87DoubleExtended;
> X86_64TargetInfo then sets LongDoubleWidth = LongDoubleAlign = 128 ;
> X86_32 TargetInfo then sets LongDoubleWidth = 96 ; LongDoubleAlign =
> 32 ;
> 
> 
> From this I can see that the patch I committed actually doesn't
> change anything but only breaks mingw x86.
> 
> 
> I can only see these values changed in Microsoft*TargetInfo classes
> which is not a parent of MINGW
> 
> 
> When I submitted the patch I just wanted to explicitly set the values
> in MinGWX86_64TargetInfo to ensure it was in fact that.
> 
> It seemed as though it was not inheriting the value from the root
> parent class somehow.
> 
> 
> 
> I was told on irc that it did fix the bug which is even stranger.
> I'm actually at a bit of a loss as to what the proper fix to this is
> then.
> 
> Apologies for breaking mingw i686 long double.
> 
> 
> I will do up a test case and try to find the actual cause of the long
> double bug and reopen the issue.
> 
> 
> 
> 
> 
> On Thu, Aug 20, 2015 at 3:09 PM, Yaron Keren < yaron.ke...@gmail.com
> > wrote:
> 
> 
> 
> 
> Hi, I've just done this exactly this in r245618 (32 bit) and r245620
> (64 bits).
> 
> 
> mingw i686 long double values were correct before r245084 and wrong
> after it.
> mingw x86_64 long double values were not modified at all by r245084
> for the reason you stated, so I agree and do not see how this
> non-change can solve anything . There is no testcase for PR24398 nor
> the OP reporting the problem was actually solved. Martell?
> 
> 
> About PR24398, long double support was in clang long ago and b oth
> code examples compile and run correctly with current svn (without
> r245084) and gcc version 5.1.0 (i686-posix-dwarf-rev0, Built by
> MinGW-W64 project).
> It's not x86_64 but as said r245084 didn't actually modify x86_64
> configuration.
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 2015-08-21 0:52 GMT+03:00 Richard Smith < rich...@metafoo.co.uk > :
> 
> 
> OK, so here's the problem:
> 
> 
> The right way to fix this seems to be to delete the assignments to
> LongDouble* from the MinGWX86_32TargetInfo constructor; the
> X86_32TargetInfo and X86TargetInfo base classes already set them to
> the right values. Likewise we can delete the assignments to
> LongDouble* from the MinGWX86_64TargetInfo constructor for the same
> reason.
> 
> 
> But... that completely reverts Martell's r245084, which apparently
> fixed PR24398. So you two need to figure out what the actual problem
> is here, and what the right fix is. r245084 didn't provide any test
> cases, and had no apparent effect other than to break long double
> for mingw32; did it really fix PR24398 (and if so, how)?
> 
> 
> 
> 
> 
> On Thu, Aug 20, 2015 at 2:27 PM, Yaron Keren < yaron.ke...@gmail.com
> > wrote:
> 
> 
> 
> 
> OK, based on testing, mingw i686 aligns long doubles to 4 bytes:
> 
> 
> 
> 
> sh-4.3$ cat < a.cpp
> #include 
> int main() {
> struct {
> char c[1];
> long double d;
> } s;
> std::coutstd::endl;
> std::coutstd::endl;
> }
> sh-4.3$ g++ a.cpp&&./a.exe
> 0x28fea0
> 0x28fea4
> 
> 
> I'll fix that.
> 
> 
> 
> 
> 
> 
> 
> 2015-08-21 0:13 GMT+03:00 Richard Smith < rich...@metafoo.co.uk > :
> 
> 
> 
> 
> 
> 
> On Wed, Aug 19, 2015 at 11:42 AM, Yaron Keren < yaron.ke...@gmail.com
> > wrote:
> 
> 
> 
> 
> Yes, it looks like a legacy issue. Documentation says so:
> 
> 
> 
> https://gcc.gnu.org/onlinedocs/gcc-4.8.2/gcc/i386-and-x86-64-Options.html
> 
> 
> 
> -m96bit-long-double -m128bit-long-double These switches control the
> size 

Re: [PATCH] D12967: fix comments

[Alexander Kornienko via cfe-commits]

alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.

LG. Thanks for the patch!

Do you need me to commit this?


http://reviews.llvm.org/D12967



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r245459 - According to i686 ABI, long double size on x86 is 12 bytes not 16 bytes.

[Yaron Keren via cfe-commits]

I can't reproduce sizeof = 12 when targetting 64 bits, it is 16
(LongDoubleWidth = 128) and have been this way since r55036.  It may be
that both 32 bit mingw and 64 bit mingw are installed and clang is
targetting the 32 bits while gcc targets 64 bits.



2015-09-19 15:24 GMT+03:00 Martell Malone :

> For reference It was raised again here also
> https://github.com/Alexpux/MINGW-packages/issues/722
> 
>
> "that is extended from X86TargetInfo
> Which sets LongDoubleFormat = ::APFloat::x87DoubleExtended;"
>
> On Saturday, September 19, 2015, Yaron Keren 
> wrote:
>
>> Thanks, I have replied there.
>>
>> 2015-09-19 13:33 GMT+03:00 Hal Finkel :
>>
>>> FYI: https://llvm.org/bugs/show_bug.cgi?id=24398 was just reopened
>>> pointing to a lack of resolution here.
>>>
>>>  -Hal
>>>
>>> - Original Message -
>>> > From: "Yaron Keren via cfe-commits" 
>>> > To: "Martell Malone" 
>>> > Cc: "Richard Smith" , "cfe-commits" <
>>> cfe-commits@lists.llvm.org>
>>> > Sent: Friday, August 21, 2015 2:47:50 AM
>>> > Subject: Re: r245459 - According to i686 ABI, long double size on x86
>>> is 12 bytes not 16 bytes.
>>> >
>>> >
>>> >
>>> >
>>> > The testcase from r245459 was not reverted and still in SVN.
>>> >
>>> >
>>> >
>>> >
>>> >
>>> > 2015-08-21 2:05 GMT+03:00 Martell Malone < martellmal...@gmail.com >
>>> > :
>>> >
>>> >
>>> >
>>> >
>>> >
>>> > I feel very silly now.
>>> > After testing the testcase again on svn it still works.
>>> > It appears the OP was looking for this patch to go onto the 3.6
>>> > branch and was applying my patch to that.
>>> >
>>> >
>>> > I'll know in future to recheck the testcase afterwards myself in
>>> > future.
>>> >
>>> >
>>> > Apologies for the noise guys.
>>> >
>>> >
>>> > Yaron I think the test case from r245459 would be useful to ensure it
>>> > is never broken in the future?
>>> >
>>> > Would you be able to recommit the test case?
>>> >
>>> >
>>> >
>>> > Kind Regards
>>> >
>>> > Martell
>>> >
>>> >
>>> >
>>> >
>>> >
>>> >
>>> >
>>> > On Thu, Aug 20, 2015 at 3:57 PM, Martell Malone <
>>> > martellmal...@gmail.com > wrote:
>>> >
>>> >
>>> >
>>> >
>>> >
>>> > There is no testcase for PR24398 nor the OP reporting the problem was
>>> > actually solved. Martell?
>>> > I'm just re-looking through it now.
>>> >
>>> >
>>> > X86TargetInfo sets LongDoubleFormat =
>>> > ::APFloat::x87DoubleExtended;
>>> > X86_64TargetInfo then sets LongDoubleWidth = LongDoubleAlign = 128 ;
>>> > X86_32 TargetInfo then sets LongDoubleWidth = 96 ; LongDoubleAlign =
>>> > 32 ;
>>> >
>>> >
>>> > From this I can see that the patch I committed actually doesn't
>>> > change anything but only breaks mingw x86.
>>> >
>>> >
>>> > I can only see these values changed in Microsoft*TargetInfo classes
>>> > which is not a parent of MINGW
>>> >
>>> >
>>> > When I submitted the patch I just wanted to explicitly set the values
>>> > in MinGWX86_64TargetInfo to ensure it was in fact that.
>>> >
>>> > It seemed as though it was not inheriting the value from the root
>>> > parent class somehow.
>>> >
>>> >
>>> >
>>> > I was told on irc that it did fix the bug which is even stranger.
>>> > I'm actually at a bit of a loss as to what the proper fix to this is
>>> > then.
>>> >
>>> > Apologies for breaking mingw i686 long double.
>>> >
>>> >
>>> > I will do up a test case and try to find the actual cause of the long
>>> > double bug and reopen the issue.
>>> >
>>> >
>>> >
>>> >
>>> >
>>> > On Thu, Aug 20, 2015 at 3:09 PM, Yaron Keren < yaron.ke...@gmail.com
>>> > > wrote:
>>> >
>>> >
>>> >
>>> >
>>> > Hi, I've just done this exactly this in r245618 (32 bit) and r245620
>>> > (64 bits).
>>> >
>>> >
>>> > mingw i686 long double values were correct before r245084 and wrong
>>> > after it.
>>> > mingw x86_64 long double values were not modified at all by r245084
>>> > for the reason you stated, so I agree and do not see how this
>>> > non-change can solve anything . There is no testcase for PR24398 nor
>>> > the OP reporting the problem was actually solved. Martell?
>>> >
>>> >
>>> > About PR24398, long double support was in clang long ago and b oth
>>> > code examples compile and run correctly with current svn (without
>>> > r245084) and gcc version 5.1.0 (i686-posix-dwarf-rev0, Built by
>>> > MinGW-W64 project).
>>> > It's not x86_64 but as said r245084 didn't actually modify x86_64
>>> > configuration.
>>> >
>>> >
>>> >
>>> >
>>> >
>>> >
>>> >
>>> >
>>> >
>>> >
>>> >
>>> > 2015-08-21 0:52 GMT+03:00 Richard Smith < rich...@metafoo.co.uk > :
>>> >
>>> >
>>> > OK, so here's the problem:
>>> >
>>> >
>>> > The right way to fix this seems to be to delete the assignments to
>>> > LongDouble* from the MinGWX86_32TargetInfo constructor; the
>>> > X86_32TargetInfo and X86TargetInfo base classes already set them to
>>> > the right 

Re: [PATCH] D12961: Update clang-tidy documentation.

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

Thank you for converting the docs!

Some of the usages of "transform" and "transformation" in these docs refer to 
"clang-modernize transform", and, thus, should be replaced with "check" or 
"clang-tidy check". I tried to find all such places. See comments.



Comment at: 
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-loop-convert.rst:226
@@ +225,3 @@
+
+While most of the transform's risk analysis is dedicated to determining whether
+the iterator or container was modified within the loop, it is possible to

s/transform/check/


Comment at: 
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-loop-convert.rst:234
@@ +233,3 @@
+level since calling a member function of the container is considered `risky`.
+The transform cannot identify expressions associated with the container that 
are
+different than the one used in the loop header, therefore the transformation

s/transform/check/


Comment at: 
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-pass-by-value.rst:7
@@ +6,3 @@
+directly by value, instead of by const-reference, and then copy. This
+transformation allows the compiler to take care of choosing the best way to
+construct the copy.

s/transformation/check/


Comment at: 
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-pass-by-value.rst:37
@@ +36,3 @@
+Since `std::move()` is a library function declared in `` it may be
+necessary to add this include. The transform will add the include directive 
when
+necessary.

s/transform/check/


Comment at: 
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-replace-auto-ptr.rst:29
@@ +28,3 @@
+Since `std::move()` is a library function declared in `` it may be
+necessary to add this include. The transform will add the include directive 
when
+necessary.

s/transform/check/


Comment at: 
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-replace-auto-ptr.rst:35
@@ +34,3 @@
+* If headers modification is not activated or if a header is not allowed to be
+  changed this transform will produce broken code (compilation error), where 
the
+  the headers' code will stay unchanged while the code using them will be

s/transform/check/


Comment at: 
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-auto.rst:56
@@ +55,3 @@
+
+The transform will only replace iterator type-specifiers when all of the
+following conditions are satisfied:

s/transform/check/


Comment at: 
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-auto.rst:131
@@ +130,3 @@
+=
+* If the initializer is an explicit conversion constructor, the transform will
+  not replace the type specifier even though it would be safe to do so.

s/transform/check/


Comment at: 
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-nullptr.rst:41
@@ +40,3 @@
+
+By default this transform will only replace the ``NULL`` macro and will skip 
any
+user-defined macros that behaves like ``NULL``. The user can use the

s/transform/check/


Repository:
  rL LLVM

http://reviews.llvm.org/D12961



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12967: fix comments

[Alexander Kornienko via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL248090: [clang-tidy] Fix example comments. (authored by 
alexfh).

Changed prior to commit:
  http://reviews.llvm.org/D12967?vs=35091=35164#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D12967

Files:
  cfe/trunk/include/clang/Tooling/CommonOptionsParser.h

Index: cfe/trunk/include/clang/Tooling/CommonOptionsParser.h
===
--- cfe/trunk/include/clang/Tooling/CommonOptionsParser.h
+++ cfe/trunk/include/clang/Tooling/CommonOptionsParser.h
@@ -42,6 +42,7 @@
 /// \code
 /// #include "clang/Frontend/FrontendActions.h"
 /// #include "clang/Tooling/CommonOptionsParser.h"
+/// #include "clang/Tooling/Tooling.h"
 /// #include "llvm/Support/CommandLine.h"
 ///
 /// using namespace clang::tooling;
@@ -56,8 +57,8 @@
 /// int main(int argc, const char **argv) {
 ///   CommonOptionsParser OptionsParser(argc, argv, MyToolCategory);
 ///   ClangTool Tool(OptionsParser.getCompilations(),
-///  OptionsParser.getSourcePathListi());
-///   return Tool.run(newFrontendActionFactory());
+///  OptionsParser.getSourcePathList());
+///   return Tool.run(newFrontendActionFactory().get());
 /// }
 /// \endcode
 class CommonOptionsParser {


Index: cfe/trunk/include/clang/Tooling/CommonOptionsParser.h
===
--- cfe/trunk/include/clang/Tooling/CommonOptionsParser.h
+++ cfe/trunk/include/clang/Tooling/CommonOptionsParser.h
@@ -42,6 +42,7 @@
 /// \code
 /// #include "clang/Frontend/FrontendActions.h"
 /// #include "clang/Tooling/CommonOptionsParser.h"
+/// #include "clang/Tooling/Tooling.h"
 /// #include "llvm/Support/CommandLine.h"
 ///
 /// using namespace clang::tooling;
@@ -56,8 +57,8 @@
 /// int main(int argc, const char **argv) {
 ///   CommonOptionsParser OptionsParser(argc, argv, MyToolCategory);
 ///   ClangTool Tool(OptionsParser.getCompilations(),
-///  OptionsParser.getSourcePathListi());
-///   return Tool.run(newFrontendActionFactory());
+///  OptionsParser.getSourcePathList());
+///   return Tool.run(newFrontendActionFactory().get());
 /// }
 /// \endcode
 class CommonOptionsParser {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r248090 - [clang-tidy] Fix example comments.

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Sat Sep 19 08:01:57 2015
New Revision: 248090

URL: http://llvm.org/viewvc/llvm-project?rev=248090=rev
Log:
[clang-tidy] Fix example comments.

Patch by don hinton!

Differential revision: http://reviews.llvm.org/D12967


Modified:
cfe/trunk/include/clang/Tooling/CommonOptionsParser.h

Modified: cfe/trunk/include/clang/Tooling/CommonOptionsParser.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/CommonOptionsParser.h?rev=248090=248089=248090=diff
==
--- cfe/trunk/include/clang/Tooling/CommonOptionsParser.h (original)
+++ cfe/trunk/include/clang/Tooling/CommonOptionsParser.h Sat Sep 19 08:01:57 
2015
@@ -42,6 +42,7 @@ namespace tooling {
 /// \code
 /// #include "clang/Frontend/FrontendActions.h"
 /// #include "clang/Tooling/CommonOptionsParser.h"
+/// #include "clang/Tooling/Tooling.h"
 /// #include "llvm/Support/CommandLine.h"
 ///
 /// using namespace clang::tooling;
@@ -56,8 +57,8 @@ namespace tooling {
 /// int main(int argc, const char **argv) {
 ///   CommonOptionsParser OptionsParser(argc, argv, MyToolCategory);
 ///   ClangTool Tool(OptionsParser.getCompilations(),
-///  OptionsParser.getSourcePathListi());
-///   return Tool.run(newFrontendActionFactory());
+///  OptionsParser.getSourcePathList());
+///   return Tool.run(newFrontendActionFactory().get());
 /// }
 /// \endcode
 class CommonOptionsParser {


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12835: [X86][SSE] Replace 128-bit SSE41 PMOVSX intrinsics with native IR

[Simon Pilgrim via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL248092: [X86][SSE] Replace 128-bit SSE41 PMOVSX intrinsics 
with native IR (authored by RKSimon).

Changed prior to commit:
  http://reviews.llvm.org/D12835?vs=34646=35167#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D12835

Files:
  cfe/trunk/include/clang/Basic/BuiltinsX86.def
  cfe/trunk/lib/Headers/smmintrin.h
  cfe/trunk/test/CodeGen/builtins-x86.c
  cfe/trunk/test/CodeGen/sse41-builtins.c

Index: cfe/trunk/include/clang/Basic/BuiltinsX86.def
===
--- cfe/trunk/include/clang/Basic/BuiltinsX86.def
+++ cfe/trunk/include/clang/Basic/BuiltinsX86.def
@@ -376,12 +376,6 @@
 TARGET_BUILTIN(__builtin_ia32_pminsd128, "V4iV4iV4i", "", "sse4.1")
 TARGET_BUILTIN(__builtin_ia32_pminud128, "V4iV4iV4i", "", "sse4.1")
 TARGET_BUILTIN(__builtin_ia32_pminuw128, "V8sV8sV8s", "", "sse4.1")
-TARGET_BUILTIN(__builtin_ia32_pmovsxbd128, "V4iV16c", "", "sse4.1")
-TARGET_BUILTIN(__builtin_ia32_pmovsxbq128, "V2LLiV16c", "", "sse4.1")
-TARGET_BUILTIN(__builtin_ia32_pmovsxbw128, "V8sV16c", "", "sse4.1")
-TARGET_BUILTIN(__builtin_ia32_pmovsxdq128, "V2LLiV4i", "", "sse4.1")
-TARGET_BUILTIN(__builtin_ia32_pmovsxwd128, "V4iV8s", "", "sse4.1")
-TARGET_BUILTIN(__builtin_ia32_pmovsxwq128, "V2LLiV8s", "", "sse4.1")
 TARGET_BUILTIN(__builtin_ia32_pmovzxbd128, "V4iV16c", "", "sse4.1")
 TARGET_BUILTIN(__builtin_ia32_pmovzxbq128, "V2LLiV16c", "", "sse4.1")
 TARGET_BUILTIN(__builtin_ia32_pmovzxbw128, "V8sV16c", "", "sse4.1")
Index: cfe/trunk/test/CodeGen/builtins-x86.c
===
--- cfe/trunk/test/CodeGen/builtins-x86.c
+++ cfe/trunk/test/CodeGen/builtins-x86.c
@@ -372,12 +372,6 @@
   tmp_V4i = __builtin_ia32_pminsd128(tmp_V4i, tmp_V4i);
   tmp_V4i = __builtin_ia32_pminud128(tmp_V4i, tmp_V4i);
   tmp_V8s = __builtin_ia32_pminuw128(tmp_V8s, tmp_V8s);
-  tmp_V4i = __builtin_ia32_pmovsxbd128(tmp_V16c);
-  tmp_V2LLi = __builtin_ia32_pmovsxbq128(tmp_V16c);
-  tmp_V8s = __builtin_ia32_pmovsxbw128(tmp_V16c);
-  tmp_V2LLi = __builtin_ia32_pmovsxdq128(tmp_V4i);
-  tmp_V4i = __builtin_ia32_pmovsxwd128(tmp_V8s);
-  tmp_V2LLi = __builtin_ia32_pmovsxwq128(tmp_V8s);
   tmp_V4i = __builtin_ia32_pmovzxbd128(tmp_V16c);
   tmp_V2LLi = __builtin_ia32_pmovzxbq128(tmp_V16c);
   tmp_V8s = __builtin_ia32_pmovzxbw128(tmp_V16c);
Index: cfe/trunk/test/CodeGen/sse41-builtins.c
===
--- cfe/trunk/test/CodeGen/sse41-builtins.c
+++ cfe/trunk/test/CodeGen/sse41-builtins.c
@@ -86,42 +86,42 @@
 
 __m128i test_mm_cvtepi8_epi16(__m128i a) {
   // CHECK-LABEL: test_mm_cvtepi8_epi16
-  // CHECK: call <8 x i16> @llvm.x86.sse41.pmovsxbw(<16 x i8> {{.*}})
+  // CHECK: sext <8 x i8> {{.*}} to <8 x i16>
   // CHECK-ASM: pmovsxbw %xmm{{.*}}, %xmm{{.*}}
   return _mm_cvtepi8_epi16(a);
 }
 
 __m128i test_mm_cvtepi8_epi32(__m128i a) {
   // CHECK-LABEL: test_mm_cvtepi8_epi32
-  // CHECK: call <4 x i32> @llvm.x86.sse41.pmovsxbd(<16 x i8> {{.*}})
+  // CHECK: sext <4 x i8> {{.*}} to <4 x i32>
   // CHECK-ASM: pmovsxbd %xmm{{.*}}, %xmm{{.*}}
   return _mm_cvtepi8_epi32(a);
 }
 
 __m128i test_mm_cvtepi8_epi64(__m128i a) {
   // CHECK-LABEL: test_mm_cvtepi8_epi64
-  // CHECK: call <2 x i64> @llvm.x86.sse41.pmovsxbq(<16 x i8> {{.*}})
+  // CHECK: sext <2 x i8> {{.*}} to <2 x i64>
   // CHECK-ASM: pmovsxbq %xmm{{.*}}, %xmm{{.*}}
   return _mm_cvtepi8_epi64(a);
 }
 
 __m128i test_mm_cvtepi16_epi32(__m128i a) {
   // CHECK-LABEL: test_mm_cvtepi16_epi32
-  // CHECK: call <4 x i32> @llvm.x86.sse41.pmovsxwd(<8 x i16> {{.*}})
+  // CHECK: sext <4 x i16> {{.*}} to <4 x i32>
   // CHECK-ASM: pmovsxwd %xmm{{.*}}, %xmm{{.*}}
   return _mm_cvtepi16_epi32(a);
 }
 
 __m128i test_mm_cvtepi16_epi64(__m128i a) {
   // CHECK-LABEL: test_mm_cvtepi16_epi64
-  // CHECK: call <2 x i64> @llvm.x86.sse41.pmovsxwq(<8 x i16> {{.*}})
+  // CHECK: sext <2 x i16> {{.*}} to <2 x i64>
   // CHECK-ASM: pmovsxwq %xmm{{.*}}, %xmm{{.*}}
   return _mm_cvtepi16_epi64(a);
 }
 
 __m128i test_mm_cvtepi32_epi64(__m128i a) {
   // CHECK-LABEL: test_mm_cvtepi32_epi64
-  // CHECK: call <2 x i64> @llvm.x86.sse41.pmovsxdq(<4 x i32> {{.*}})
+  // CHECK: sext <2 x i32> {{.*}} to <2 x i64>
   // CHECK-ASM: pmovsxdq %xmm{{.*}}, %xmm{{.*}}
   return _mm_cvtepi32_epi64(a);
 }
Index: cfe/trunk/lib/Headers/smmintrin.h
===
--- cfe/trunk/lib/Headers/smmintrin.h
+++ cfe/trunk/lib/Headers/smmintrin.h
@@ -286,37 +286,37 @@
 static __inline__ __m128i __DEFAULT_FN_ATTRS
 _mm_cvtepi8_epi16(__m128i __V)
 {
-  return (__m128i) __builtin_ia32_pmovsxbw128((__v16qi) __V);
+  return (__m128i)__builtin_convertvector(__builtin_shufflevector((__v16qi)__V, (__v16qi)__V, 0, 1, 2, 3, 4, 5, 6, 7), __v8hi);
 }
 
 static __inline__ __m128i __DEFAULT_FN_ATTRS
 _mm_cvtepi8_epi32(__m128i __V)
 {
-  return (__m128i) 

r248092 - [X86][SSE] Replace 128-bit SSE41 PMOVSX intrinsics with native IR

[Simon Pilgrim via cfe-commits]

Author: rksimon
Date: Sat Sep 19 10:12:38 2015
New Revision: 248092

URL: http://llvm.org/viewvc/llvm-project?rev=248092=rev
Log:
[X86][SSE] Replace 128-bit SSE41 PMOVSX intrinsics with native IR

128-bit vector integer sign extensions correctly lower to the pmovsx 
instructions even for debug builds.

This patch removes the builtins and reimplements the _mm_cvtepi*_epi* 
intrinsics __using builtin_shufflevector (to extract the bottom most subvector) 
and __builtin_convertvector (to actually perform the sign extension).

Differential Revision: http://reviews.llvm.org/D12835

Modified:
cfe/trunk/include/clang/Basic/BuiltinsX86.def
cfe/trunk/lib/Headers/smmintrin.h
cfe/trunk/test/CodeGen/builtins-x86.c
cfe/trunk/test/CodeGen/sse41-builtins.c

Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=248092=248091=248092=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Sat Sep 19 10:12:38 2015
@@ -376,12 +376,6 @@ TARGET_BUILTIN(__builtin_ia32_pminsb128,
 TARGET_BUILTIN(__builtin_ia32_pminsd128, "V4iV4iV4i", "", "sse4.1")
 TARGET_BUILTIN(__builtin_ia32_pminud128, "V4iV4iV4i", "", "sse4.1")
 TARGET_BUILTIN(__builtin_ia32_pminuw128, "V8sV8sV8s", "", "sse4.1")
-TARGET_BUILTIN(__builtin_ia32_pmovsxbd128, "V4iV16c", "", "sse4.1")
-TARGET_BUILTIN(__builtin_ia32_pmovsxbq128, "V2LLiV16c", "", "sse4.1")
-TARGET_BUILTIN(__builtin_ia32_pmovsxbw128, "V8sV16c", "", "sse4.1")
-TARGET_BUILTIN(__builtin_ia32_pmovsxdq128, "V2LLiV4i", "", "sse4.1")
-TARGET_BUILTIN(__builtin_ia32_pmovsxwd128, "V4iV8s", "", "sse4.1")
-TARGET_BUILTIN(__builtin_ia32_pmovsxwq128, "V2LLiV8s", "", "sse4.1")
 TARGET_BUILTIN(__builtin_ia32_pmovzxbd128, "V4iV16c", "", "sse4.1")
 TARGET_BUILTIN(__builtin_ia32_pmovzxbq128, "V2LLiV16c", "", "sse4.1")
 TARGET_BUILTIN(__builtin_ia32_pmovzxbw128, "V8sV16c", "", "sse4.1")

Modified: cfe/trunk/lib/Headers/smmintrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/smmintrin.h?rev=248092=248091=248092=diff
==
--- cfe/trunk/lib/Headers/smmintrin.h (original)
+++ cfe/trunk/lib/Headers/smmintrin.h Sat Sep 19 10:12:38 2015
@@ -286,37 +286,37 @@ _mm_cmpeq_epi64(__m128i __V1, __m128i __
 static __inline__ __m128i __DEFAULT_FN_ATTRS
 _mm_cvtepi8_epi16(__m128i __V)
 {
-  return (__m128i) __builtin_ia32_pmovsxbw128((__v16qi) __V);
+  return 
(__m128i)__builtin_convertvector(__builtin_shufflevector((__v16qi)__V, 
(__v16qi)__V, 0, 1, 2, 3, 4, 5, 6, 7), __v8hi);
 }
 
 static __inline__ __m128i __DEFAULT_FN_ATTRS
 _mm_cvtepi8_epi32(__m128i __V)
 {
-  return (__m128i) __builtin_ia32_pmovsxbd128((__v16qi) __V);
+  return 
(__m128i)__builtin_convertvector(__builtin_shufflevector((__v16qi)__V, 
(__v16qi)__V, 0, 1, 2, 3), __v4si);
 }
 
 static __inline__ __m128i __DEFAULT_FN_ATTRS
 _mm_cvtepi8_epi64(__m128i __V)
 {
-  return (__m128i) __builtin_ia32_pmovsxbq128((__v16qi) __V);
+  return 
(__m128i)__builtin_convertvector(__builtin_shufflevector((__v16qi)__V, 
(__v16qi)__V, 0, 1), __v2di);
 }
 
 static __inline__ __m128i __DEFAULT_FN_ATTRS
 _mm_cvtepi16_epi32(__m128i __V)
 {
-  return (__m128i) __builtin_ia32_pmovsxwd128((__v8hi) __V);
+  return (__m128i)__builtin_convertvector(__builtin_shufflevector((__v8hi)__V, 
(__v8hi)__V, 0, 1, 2, 3), __v4si);
 }
 
 static __inline__ __m128i __DEFAULT_FN_ATTRS
 _mm_cvtepi16_epi64(__m128i __V)
 {
-  return (__m128i) __builtin_ia32_pmovsxwq128((__v8hi)__V);
+  return (__m128i)__builtin_convertvector(__builtin_shufflevector((__v8hi)__V, 
(__v8hi)__V, 0, 1), __v2di);
 }
 
 static __inline__ __m128i __DEFAULT_FN_ATTRS
 _mm_cvtepi32_epi64(__m128i __V)
 {
-  return (__m128i) __builtin_ia32_pmovsxdq128((__v4si)__V);
+  return (__m128i)__builtin_convertvector(__builtin_shufflevector((__v4si)__V, 
(__v4si)__V, 0, 1), __v2di);
 }
 
 /* SSE4 Packed Integer Zero-Extension.  */

Modified: cfe/trunk/test/CodeGen/builtins-x86.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/builtins-x86.c?rev=248092=248091=248092=diff
==
--- cfe/trunk/test/CodeGen/builtins-x86.c (original)
+++ cfe/trunk/test/CodeGen/builtins-x86.c Sat Sep 19 10:12:38 2015
@@ -372,12 +372,6 @@ void f0() {
   tmp_V4i = __builtin_ia32_pminsd128(tmp_V4i, tmp_V4i);
   tmp_V4i = __builtin_ia32_pminud128(tmp_V4i, tmp_V4i);
   tmp_V8s = __builtin_ia32_pminuw128(tmp_V8s, tmp_V8s);
-  tmp_V4i = __builtin_ia32_pmovsxbd128(tmp_V16c);
-  tmp_V2LLi = __builtin_ia32_pmovsxbq128(tmp_V16c);
-  tmp_V8s = __builtin_ia32_pmovsxbw128(tmp_V16c);
-  tmp_V2LLi = __builtin_ia32_pmovsxdq128(tmp_V4i);
-  tmp_V4i = __builtin_ia32_pmovsxwd128(tmp_V8s);
-  tmp_V2LLi = __builtin_ia32_pmovsxwq128(tmp_V8s);
   tmp_V4i = 

[PATCH] D12993: [analyzer] Add TK_EntireMemSpace invalidation trait.

[Devin Coughlin via cfe-commits]

dcoughlin created this revision.
dcoughlin added reviewers: krememek, zaks.anna.
dcoughlin added subscribers: cfe-commits, seaneveson.

This is a patch to support Sean Eveson's work on loop widening. It adds a new
TK_EntireMemSpace invalidation trait that, when applied to a MemSpaceRegion, 
indicates
that the entire memory space should be invalidated.

Clients would add this trait before invalidating. For example:

```
RegionAndSymbolInvalidationTraits ITraits;
ITraits.setTrait(MRMgr.getStackLocalsRegion(STC),
 RegionAndSymbolInvalidationTraits::TK_EntireMemSpace);

```

This patch updates the existing logic invalidating global memspace regions for 
calls to
additionally handle arbitrary memspaces. When generating initial clusters during
cluster analysis we will now add a cluster to the worklist if the memspace for 
its
base is marked with TK_EntireMemSpace.

The patch also moves the logic for invalidating globals from ClusterAnalysis to
invalidateRegionsWorker so that it is not shared with removeDeadBindingsWorker.

There are no explicit tests with this patch -- but when applied to Sean's patch 
in
http://reviews.llvm.org/D12358 and after updating his code to set the trait,
the failing tests in that patch now pass.

http://reviews.llvm.org/D12993

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  lib/StaticAnalyzer/Core/RegionStore.cpp

Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -650,35 +650,25 @@
 
   RegionBindingsRef B;
 
-private:
-  GlobalsFilterKind GlobalsFilter;
 
 protected:
   const ClusterBindings *getCluster(const MemRegion *R) {
 return B.lookup(R);
   }
 
-  /// Returns true if the memory space of the given region is one of the global
-  /// regions specially included at the start of analysis.
-  bool isInitiallyIncludedGlobalRegion(const MemRegion *R) {
-switch (GlobalsFilter) {
-case GFK_None:
-  return false;
-case GFK_SystemOnly:
-  return isa(R->getMemorySpace());
-case GFK_All:
-  return isa(R->getMemorySpace());
-}
-
-llvm_unreachable("unknown globals filter");
+  /// Returns true if all clusters in the given memspace should be initially
+  /// included in the cluster analysis. Subclasses may provide their
+  /// own implementation.
+  bool includeEntireMemorySpace(const MemRegion *Base) {
+return false;
   }
 
 public:
   ClusterAnalysis(RegionStoreManager , ProgramStateManager ,
-  RegionBindingsRef b, GlobalsFilterKind GFK)
+  RegionBindingsRef b )
 : RM(rm), Ctx(StateMgr.getContext()),
   svalBuilder(StateMgr.getSValBuilder()),
-  B(b), GlobalsFilter(GFK) {}
+  B(b) {}
 
   RegionBindingsRef getRegionBindings() const { return B; }
 
@@ -696,8 +686,9 @@
   assert(!Cluster.isEmpty() && "Empty clusters should be removed");
   static_cast(this)->VisitAddedToCluster(Base, Cluster);
 
-  // If this is an interesting global region, add it the work list up front.
-  if (isInitiallyIncludedGlobalRegion(Base))
+  // If the base's memspace should be entirely invalidated, add the cluster
+  // to the workspace up front.
+  if (static_cast(this)->includeEntireMemorySpace(Base))
 AddToWorkList(WorkListElement(Base), );
 }
   }
@@ -941,6 +932,7 @@
   InvalidatedSymbols 
   RegionAndSymbolInvalidationTraits 
   StoreManager::InvalidatedRegions *Regions;
+  GlobalsFilterKind GlobalsFilter;
 public:
   invalidateRegionsWorker(RegionStoreManager ,
   ProgramStateManager ,
@@ -951,12 +943,21 @@
   RegionAndSymbolInvalidationTraits ,
   StoreManager::InvalidatedRegions *r,
   GlobalsFilterKind GFK)
-: ClusterAnalysis(rm, stateMgr, b, GFK),
-  Ex(ex), Count(count), LCtx(lctx), IS(is), ITraits(ITraitsIn), Regions(r){}
+: ClusterAnalysis(rm, stateMgr, b),
+  Ex(ex), Count(count), LCtx(lctx), IS(is), ITraits(ITraitsIn), Regions(r),
+  GlobalsFilter(GFK) {}
 
   void VisitCluster(const MemRegion *baseR, const ClusterBindings *C);
   void VisitBinding(SVal V);
-};
+
+  /// Returns true if all clusters in the memory space for \p Base should be
+  /// be invalidated.
+  bool includeEntireMemorySpace(const MemRegion *Base);
+
+  /// Returns true if the memory space of the given region is one of the global
+  /// regions specially included at the start of invalidation.
+  bool isInitiallyIncludedGlobalRegion(const MemRegion *R);
+  };
 }
 
 void invalidateRegionsWorker::VisitBinding(SVal V) {
@@ -1085,6 +1086,29 @@
   B = B.addBinding(baseR, BindingKey::Direct, V);
 }
 
+bool invalidateRegionsWorker::isInitiallyIncludedGlobalRegion(
+const MemRegion *R) {
+  switch (GlobalsFilter) {
+  case GFK_None:
+return false;
+  case GFK_SystemOnly:
+

Re: [PATCH] D12358: [Analyzer] Handling constant bound loops

[Devin Coughlin via cfe-commits]

dcoughlin added a comment.

There is a patch to add memspace region invalidation in 
http://reviews.llvm.org/D12993.


http://reviews.llvm.org/D12358



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits