[PATCH] D50078: clang-format: support aligned nested conditionals formatting

2020-05-21 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

I think the new formatting is unquestionably better than the old formatting.

But...

This change means that the output from clang-format-11 can't be made compatible 
with older versions of clang-format, which is also unfortunate.

IMHO it should be a goal to have "breaking" format changes like this always be 
opt-in via a new setting (or a new option for an existing setting).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D50078/new/

https://reviews.llvm.org/D50078



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

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

2020-05-26 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

> And I also think it should be on by default instead of modifying many 
> .clang-format files. So IMHO if we add an option, it should be opt-out.

I can live with it being opt-out. My big concern is that (as it currently 
stands) our project gets different 'canonical' clang-format results depending 
on whether people are using LLVM10 vs LLVM11 (and both are common and supported 
by our project), which is a headache. I just want a way to get consistent, 
predictable formatting regardless of the clang-format version.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D50078/new/

https://reviews.llvm.org/D50078



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

[PATCH] D34654: Allow passing a regex for headers to exclude from clang-tidy

2020-10-13 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

This is marked as "needs revision", but it's not clear to me what the requested 
changes actually are.

The proposed fix here may not be perfect, but AFAICT, there is simply no good 
way to exclude headers that are beyond your control at present.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D34654/new/

https://reviews.llvm.org/D34654

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

[PATCH] D134224: [clang][modules][deps] Report modulemaps describing excluded headers

2022-09-23 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

This change has broken x86-32 builds, at least on Linux.

  
/usr/local/google/home/srj/GitHub/llvm-project/16/llvm/include/llvm/ADT/PointerIntPair.h:136:25:
 error: static assertion failed: PointerIntPair with integer size too large for 
pointer
136 |   static_assert(IntBits <= PtrTraits::NumLowBitsAvailable,
| ^
  
/usr/local/google/home/srj/GitHub/llvm-project/16/llvm/include/llvm/ADT/PointerIntPair.h:136:25:
 note: the comparison reduces to ‘(3 <= 2)’
  
/usr/local/google/home/srj/GitHub/llvm-project/16/llvm/include/llvm/ADT/PointerIntPair.h:151:42:
 warning: left shift count >= width of type [-Wshift-count-overflow]
151 | ShiftedIntMask = (uintptr_t)(IntMask << IntShift)
| ~^~~~
  
/usr/local/google/home/srj/GitHub/llvm-project/16/llvm/include/llvm/ADT/PointerIntPair.h:151:42:
 error: right operand of shift expression ‘(7 << 4294967295)’ is greater than 
or equal to the precision 32 of the left operand [-fpermissive]
  
/usr/local/google/home/srj/GitHub/llvm-project/16/llvm/include/llvm/ADT/PointerIntPair.h:138:8:
 error: enumerator value for ‘ShiftedIntMask’ is not an integer constant
138 |   enum MaskAndShiftConstants : uintptr_t {

Please fix or revert immediately.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134224/new/

https://reviews.llvm.org/D134224

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

[PATCH] D134653: [clang][modules] Over-align the `Module` class

2022-09-26 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

How did the original change pass presubmit testing -- is LLVM no longer testing 
on 32-bit targets?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134653/new/

https://reviews.llvm.org/D134653

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

[PATCH] D134653: [clang][modules] Over-align the `Module` class

2022-09-26 Thread Steven Johnson via Phabricator via cfe-commits 
srj added inline comments.



Comment at: clang/include/clang/Basic/Module.h:96
 /// Describes a module or submodule.
-class Module {
+class alignas(8) Module {
 public:

Probably worth a comment here, it's not necessarily obvious to the reader that 
this is to fix a 32-bit pointer issue.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134653/new/

https://reviews.llvm.org/D134653

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

[PATCH] D134653: [clang][modules] Over-align the `Module` class

2022-09-26 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

In D134653#3815627 , @jansvoboda11 
wrote:

> Pre-commit CI only runs on x64 Debian and x64 Windows. I don't have good way 
> to reproduce the issue, so ideally the person who reported it (@ms178?) steps 
> up.

I'm the one who reported the breakage.

Regarding presubmit testing, that's of great concern to me; if LLVM isn't 
testing on x86-32 then it's not really *supporting* x86-32 anymore (as a 
compilation host anyway), and that's a rather large change to make without 
warning. Do you know to whom I should escalate that concern?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134653/new/

https://reviews.llvm.org/D134653

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

[PATCH] D134653: [clang][modules] Over-align the `Module` class

2022-09-26 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

In D134653#3815627 , @jansvoboda11 
wrote:

> Pre-commit CI only runs on x64 Debian and x64 Windows. I don't have good way 
> to reproduce the issue, so ideally the person who reported it (@ms178?) steps 
> up.

FYI, adding this patch to my local build does allow me to *compile* properly 
now. I haven't tested for correctness.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134653/new/

https://reviews.llvm.org/D134653

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

[PATCH] D134653: [clang][modules] Over-align the `Module` class

2022-09-26 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

> Thanks for verifying the build. You can test for correctness by "building" 
> the `check-clang` target.

Thanks -- I'll also build and test Halide locally.

> You can contact the infrastructure working group. Note that we run more 
> configurations post-commit: https://lab.llvm.org/buildbot/#/console

I opened a question at https://discourse.llvm.org/t/x86-32-bit-testing/65480 on 
Friday but got no responses. Is there a better venue for asking the right 
people?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134653/new/

https://reviews.llvm.org/D134653

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

[PATCH] D134653: [clang][modules] Over-align the `Module` class

2022-09-26 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

> Thanks for verifying the build. You can test for correctness by "building" 
> the `check-clang` target.

Tests are passing locally, LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134653/new/

https://reviews.llvm.org/D134653

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

[PATCH] D141861: [nvptx-arch] Dynamically load the CUDA runtime if not found during the build

2023-01-30 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

In D141861#4060100 , @jhuber6 wrote:

> In D141861#4060028 , @srj wrote:
>
>> This change appears to have broken the build when crosscompiling to x86-32 
>> on a Linux x86-64 system; on the Halide buildbots, we now fail at link time 
>> with
>>
>>   FAILED: bin/nvptx-arch 
>>   : && /usr/bin/g++-7  -m32 -Wno-psabi -fPIC -fno-semantic-interposition 
>> -fvisibility-inlines-hidden -Werror=date-time -Wall -Wextra 
>> -Wno-unused-parameter -Wwrite-strings -Wcast-qual 
>> -Wno-missing-field-initializers -pedantic -Wno-long-long 
>> -Wimplicit-fallthrough -Wno-maybe-uninitialized -Wno-noexcept-type 
>> -Wdelete-non-virtual-dtor -Wno-comment -Wno-misleading-indentation 
>> -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common 
>> -Woverloaded-virtual -fno-strict-aliasing -O3 -DNDEBUG 
>> -Wl,-rpath-link,/home/halidenightly/build_bot/worker/llvm-16-x86-32-linux/llvm-build/./lib
>>   -Wl,--gc-sections 
>> tools/clang/tools/nvptx-arch/CMakeFiles/nvptx-arch.dir/NVPTXArch.cpp.o -o 
>> bin/nvptx-arch  -Wl,-rpath,"\$ORIGIN/../lib"  lib/libLLVMSupport.a  
>> -lpthread  -lrt  -ldl  -lpthread  -lm  lib/libLLVMDemangle.a && :
>>   /usr/bin/ld: 
>> tools/clang/tools/nvptx-arch/CMakeFiles/nvptx-arch.dir/NVPTXArch.cpp.o: in 
>> function `handleError(cudaError_enum)':
>>   NVPTXArch.cpp:(.text._ZL11handleError14cudaError_enum+0x2b): undefined 
>> reference to `cuGetErrorString'
>>   /usr/bin/ld: 
>> tools/clang/tools/nvptx-arch/CMakeFiles/nvptx-arch.dir/NVPTXArch.cpp.o: in 
>> function `main':
>>   NVPTXArch.cpp:(.text.startup.main+0xcf): undefined reference to `cuInit'
>>   /usr/bin/ld: NVPTXArch.cpp:(.text.startup.main+0xf9): undefined reference 
>> to `cuDeviceGetCount'
>>   /usr/bin/ld: NVPTXArch.cpp:(.text.startup.main+0x11e): undefined reference 
>> to `cuDeviceGet'
>>   /usr/bin/ld: NVPTXArch.cpp:(.text.startup.main+0x131): undefined reference 
>> to `cuDeviceGetAttribute'
>>   /usr/bin/ld: NVPTXArch.cpp:(.text.startup.main+0x146): undefined reference 
>> to `cuDeviceGetAttribute'
>>   collect2: error: ld returned 1 exit status
>>
>> I'm guessing that the problem here is that the build machine has Cuda 
>> installed (so the headers are found), but no 32-bit version of Cuda (so 
>> linking fails).
>>
>> Probably easy to fix, but as of right now, our 32-bit testing is dead in the 
>> water; could someone please revert this pending a proper fix?
>
> Can you let me know if rG4ce454c654bd 
>  solves 
> it? I'm guessing the problem is arising when we find the libraries at build 
> configure time but not at build time so we might need another check as well.

It looks like this change (but not the rG4ce454c654bd 
) is in 
the 17 branch, as the latter is now failing in the same way for crosscompiles.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141861/new/

https://reviews.llvm.org/D141861

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

[PATCH] D141861: [nvptx-arch] Dynamically load the CUDA runtime if not found during the build

2023-01-30 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

In D141861#4091403 , @jhuber6 wrote:

> In D141861#4091383 , @srj wrote:
>
>> It looks like this change (but not the rG4ce454c654bd 
>> ) is in 
>> the 17 branch, as the latter is now failing in the same way for 
>> crosscompiles.
>
> It looks like it's there for me, see 
> https://github.com/llvm/llvm-project/blob/main/clang/tools/nvptx-arch/NVPTXArch.cpp#L20.
>  What is the issue? I made a slight tweak a few days ago on the 17 branch 
> that updated how we could the CUDA driver.

We just started testing with the 17 branch this morning, can you point me at 
your tweak?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141861/new/

https://reviews.llvm.org/D141861

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

[PATCH] D141861: [nvptx-arch] Dynamically load the CUDA runtime if not found during the build

2023-01-30 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

> https://github.com/llvm/llvm-project/commit/759dec253695f38a101c74905c819ea47392e515.
>  Does it work if you revert this? I wouldn't think it wouldn't affect 
> anything. That's the only change that happened after the 16 release as far as 
> I'm aware.

Reverting this (well, actually just monkey-patching in the old code) does 
indeed make things build correctly again -- a bit surprising to me too, but 
that's CMake for you. Reverting the change seems appropriate pending a tested 
fix.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141861/new/

https://reviews.llvm.org/D141861

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

[PATCH] D141861: [nvptx-arch] Dynamically load the CUDA runtime if not found during the build

2023-01-30 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

In D141861#4091869 , @jhuber6 wrote:

> In D141861#4091851 , @srj wrote:
>
>>> https://github.com/llvm/llvm-project/commit/759dec253695f38a101c74905c819ea47392e515.
>>>  Does it work if you revert this? I wouldn't think it wouldn't affect 
>>> anything. That's the only change that happened after the 16 release as far 
>>> as I'm aware.
>>
>> Reverting this (well, actually just monkey-patching in the old code) does 
>> indeed make things build correctly again -- a bit surprising to me too, but 
>> that's CMake for you. Reverting the change seems appropriate pending a 
>> tested fix.
>
> That's bizarre, that means it's finding `CUDAToolkit` but can't link against 
> it?

It's finding a 64-bit CUDAToolkit, which it can't link against because the rest 
of the build is 32-bit.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141861/new/

https://reviews.llvm.org/D141861

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

[PATCH] D141861: [nvptx-arch] Dynamically load the CUDA runtime if not found during the build

2023-01-30 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

In D141861#4091903 , @jhuber6 wrote:

> In D141861#4091897 , @srj wrote:
>
>> It's finding a 64-bit CUDAToolkit, which it can't link against because the 
>> rest of the build is 32-bit.
>
> Wondering why it didn't find it before then. But that's definitely a weird 
> configuration. Not sure what a good generic solution is. We could always make 
> it `dlopen` all the time.

Crosscompiling to x86-32 on an x86-64 host doesn't strike me as particularly 
weird at all (especially on Windows), but apparently it is quite weird for LLVM 
at this point in time as we keep getting a lot of different things broken there 
:-)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141861/new/

https://reviews.llvm.org/D141861

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

[PATCH] D141861: [nvptx-arch] Dynamically load the CUDA runtime if not found during the build

2023-01-30 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

In D141861#4091949 , @jhuber6 wrote:

> In D141861#4091922 , @srj wrote:
>
>> Crosscompiling to x86-32 on an x86-64 host doesn't strike me as particularly 
>> weird at all (especially on Windows), but apparently it is quite weird for 
>> LLVM at this point in time as we keep getting a lot of different things 
>> broken there :-)
>
> I'm not very familiar with this type of build. Are there any variables we 
> could pick up to just disable this if it's not building for the host system? 
> Something like `CMAKE_CROSSCOMPILING`?

I'm not an expert on the LLVM build system, so I'm not entirely sure, but I'd 
start by examining the CMake setting `LLVM_BUILD_32_BITS` (which we set to ON 
in this case)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141861/new/

https://reviews.llvm.org/D141861

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

[PATCH] D141861: [nvptx-arch] Dynamically load the CUDA runtime if not found during the build

2023-01-30 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

In D141861#4091987 , @jhuber6 wrote:

> Can you let me know if adding this fixes it.

Unfortunately, no.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141861/new/

https://reviews.llvm.org/D141861

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

[PATCH] D141861: [nvptx-arch] Dynamically load the CUDA runtime if not found during the build

2023-01-30 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

In D141861#4092034 , @srj wrote:

> In D141861#4091987 , @jhuber6 wrote:
>
>> Can you let me know if adding this fixes it.
>
> Unfortunately, no. (That is: It does not fix it. CMAKE_CROSSCOMPILING is 
> unfortunately a subtle and flaky beast, see e.g. 
> https://gitlab.kitware.com/cmake/cmake/-/issues/21744)

Update: I may have a way to make this work from my side; testing now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141861/new/

https://reviews.llvm.org/D141861

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

[PATCH] D141861: [nvptx-arch] Dynamically load the CUDA runtime if not found during the build

2023-01-30 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

In D141861#4092096 , @srj wrote:

> Update: I may have a way to make this work from my side; testing now.

Alas, that didn't work, stlll broken.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141861/new/

https://reviews.llvm.org/D141861

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

[PATCH] D141861: [nvptx-arch] Dynamically load the CUDA runtime if not found during the build

2023-01-30 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

In D141861#4092190 , @jhuber6 wrote:

> In D141861#4092182 , @srj wrote:
>
>> In D141861#4092096 , @srj wrote:
>>
>>> Update: I may have a way to make this work from my side; testing now.
>>
>> Alas, that didn't work, stlll broken.
>
> Interesting. It's definitely a bad problem that we find the 64-bit version 
> and try to link with it for a cross-compile. I don't want to revert the code 
> because the old `FIND_CUDA` is officially deprecated. I figured there was 
> surely a way to check if LLVM was cross compiling.

I'll see if I can get our CMake expert to weigh in :-)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141861/new/

https://reviews.llvm.org/D141861

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

[PATCH] D141861: [nvptx-arch] Dynamically load the CUDA runtime if not found during the build

2023-01-31 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

In D141861#4092237 , @tra wrote:

> For what it's worth, NVIDIA has started deprecating 32-bit binaries long ago 
> (https://forums.developer.nvidia.com/t/deprecation-plans-for-32-bit-linux-x86-cuda-toolkit-and-cuda-driver/31356)
>   and the process had finally come to the end with the release of CUDA-12:

Hmm... maybe the right answer then is to just always use the dynamic-loading 
path when doing any kind of 32-bit build.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141861/new/

https://reviews.llvm.org/D141861

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

[PATCH] D141861: [nvptx-arch] Dynamically load the CUDA runtime if not found during the build

2023-01-31 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

In D141861#4094043 , @jhuber6 wrote:

> Would this just require checking `LLVM_BUILD_32_BITS`? Should be an easy 
> change.

I think so. (It might be tempting to check `if (CMAKE_SIZEOF_VOID_P EQUAL 8)` 
but LLVM_BUILD_32_BITS is likely to be a more explicit signal.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141861/new/

https://reviews.llvm.org/D141861

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

[PATCH] D141861: [nvptx-arch] Dynamically load the CUDA runtime if not found during the build

2023-01-31 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

In D141861#4094059 , @jhuber6 wrote:

> In D141861#4094058 , @srj wrote:
>
>> In D141861#4094043 , @jhuber6 
>> wrote:
>>
>>> Would this just require checking `LLVM_BUILD_32_BITS`? Should be an easy 
>>> change.
>>
>> I think so. (It might be tempting to check `if (CMAKE_SIZEOF_VOID_P EQUAL 
>> 8)` but LLVM_BUILD_32_BITS is likely to be a more explicit signal.)
>
> SG, want me to push that real quick?

Yes please!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141861/new/

https://reviews.llvm.org/D141861

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

[PATCH] D141861: [nvptx-arch] Dynamically load the CUDA runtime if not found during the build

2023-01-31 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

In D141861#4094079 , @jhuber6 wrote:

> In D141861#4094063 , @srj wrote:
>
>> Yes please!
>
> Let me know if this fixes anything rG9f64fbb882dc 
> .

Testing now


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141861/new/

https://reviews.llvm.org/D141861

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

[PATCH] D141861: [nvptx-arch] Dynamically load the CUDA runtime if not found during the build

2023-01-31 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

In D141861#4094084 , @srj wrote:

> In D141861#4094079 , @jhuber6 wrote:
>
>> In D141861#4094063 , @srj wrote:
>>
>>> Yes please!
>>
>> Let me know if this fixes anything rG9f64fbb882dc 
>> .
>
> Testing now

So far, so good, let me just verify a few more things


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141861/new/

https://reviews.llvm.org/D141861

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

[PATCH] D141861: [nvptx-arch] Dynamically load the CUDA runtime if not found during the build

2023-01-31 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

In D141861#4094238 , @srj wrote:

> In D141861#4094084 , @srj wrote:
>
>> In D141861#4094079 , @jhuber6 
>> wrote:
>>
>>> In D141861#4094063 , @srj wrote:
>>>
 Yes please!
>>>
>>> Let me know if this fixes anything rG9f64fbb882dc 
>>> .
>>
>> Testing now
>
> So far, so good, let me just verify a few more things

Looks good! Thanks for the fix!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141861/new/

https://reviews.llvm.org/D141861

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

[PATCH] D141051: [CUDA][HIP] Add support for `--offload-arch=native` to CUDA and refactor

2023-01-12 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

For reasons that aren't yet clear to me, this change is failing to compile when 
using gcc-7 and targeting 32-bit targets; the error is of the form

  AMDGPU.cpp:773:10: error: could not convert ‘GPUArchs’ from 
‘llvm::SmallVector, 1>’ to 
‘llvm::Expected > >’
 return GPUArchs;


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141051/new/

https://reviews.llvm.org/D141051

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

[PATCH] D141861: [nvptx-arch] Dynamically load the CUDA runtime if not found during the build

2023-01-17 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

This change appears to have broken the build when crosscompiling to x86-32 on a 
Linux x86-64 system; on the Halide buildbots, we now fail at link time with

  FAILED: bin/nvptx-arch 
  : && /usr/bin/g++-7  -m32 -Wno-psabi -fPIC -fno-semantic-interposition 
-fvisibility-inlines-hidden -Werror=date-time -Wall -Wextra 
-Wno-unused-parameter -Wwrite-strings -Wcast-qual 
-Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough 
-Wno-maybe-uninitialized -Wno-noexcept-type -Wdelete-non-virtual-dtor 
-Wno-comment -Wno-misleading-indentation -fdiagnostics-color 
-ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual 
-fno-strict-aliasing -O3 -DNDEBUG 
-Wl,-rpath-link,/home/halidenightly/build_bot/worker/llvm-16-x86-32-linux/llvm-build/./lib
  -Wl,--gc-sections 
tools/clang/tools/nvptx-arch/CMakeFiles/nvptx-arch.dir/NVPTXArch.cpp.o -o 
bin/nvptx-arch  -Wl,-rpath,"\$ORIGIN/../lib"  lib/libLLVMSupport.a  -lpthread  
-lrt  -ldl  -lpthread  -lm  lib/libLLVMDemangle.a && :
  /usr/bin/ld: 
tools/clang/tools/nvptx-arch/CMakeFiles/nvptx-arch.dir/NVPTXArch.cpp.o: in 
function `handleError(cudaError_enum)':
  NVPTXArch.cpp:(.text._ZL11handleError14cudaError_enum+0x2b): undefined 
reference to `cuGetErrorString'
  /usr/bin/ld: 
tools/clang/tools/nvptx-arch/CMakeFiles/nvptx-arch.dir/NVPTXArch.cpp.o: in 
function `main':
  NVPTXArch.cpp:(.text.startup.main+0xcf): undefined reference to `cuInit'
  /usr/bin/ld: NVPTXArch.cpp:(.text.startup.main+0xf9): undefined reference to 
`cuDeviceGetCount'
  /usr/bin/ld: NVPTXArch.cpp:(.text.startup.main+0x11e): undefined reference to 
`cuDeviceGet'
  /usr/bin/ld: NVPTXArch.cpp:(.text.startup.main+0x131): undefined reference to 
`cuDeviceGetAttribute'
  /usr/bin/ld: NVPTXArch.cpp:(.text.startup.main+0x146): undefined reference to 
`cuDeviceGetAttribute'
  collect2: error: ld returned 1 exit status


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141861/new/

https://reviews.llvm.org/D141861

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

[PATCH] D143813: [ClangFE] Check that __sync builtins are naturally aligned.

2023-03-16 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

> (I certainly agree everyone should be using __atomic and not __sync.)

Halide still uses the `__sync` builtins for 32-bit targets, for reasons I don't 
have at hand now (IIRC there were downstream users with tooling issues, I will 
re-check). (I don't think we are using `__sync` for 64-bit targets; if we are, 
that's a mistake on our part.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143813/new/

https://reviews.llvm.org/D143813

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

[PATCH] D146987: [Assignment Tracking] Enable by default

2023-04-20 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

For the record, this also breaks (broke?) Halide:

Assertion failed: (!(Elmt.getFragmentOrDefault() == 
Next.getFragmentOrDefault())), function operator(), file 
AssignmentTrackingAnalysis.cpp, line 2020.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146987/new/

https://reviews.llvm.org/D146987

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

[PATCH] D146987: [Assignment Tracking] Enable by default

2023-04-20 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

In D146987#4285299 , @jmorse wrote:

> In D146987#4285275 , @srj wrote:
>
>> For the record, this also breaks (broke?) Halide:
>>
>> Assertion failed: (!(Elmt.getFragmentOrDefault() == 
>> Next.getFragmentOrDefault())), function operator(), file 
>> AssignmentTrackingAnalysis.cpp, line 2020.
>
> That's a new one -- would you be able to give some context and a reproducer? 
> Thanks for reporting!

Yep -- as of LLVM commit 3c9083f6757cbaf6f8d6c601586d99a11faf642e 
, Halide 
is still broken. Working on a repro case now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146987/new/

https://reviews.llvm.org/D146987

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

[PATCH] D146987: [Assignment Tracking] Enable by default

2023-04-20 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

>> That's a new one -- would you be able to give some context and a reproducer? 
>> Thanks for reporting!
>
> Yep -- as of LLVM commit 3c9083f6757cbaf6f8d6c601586d99a11faf642e 
> , Halide 
> is still broken. Working on a repro case now.

This is regrettably large, but easy to repro:

- Unzip the file
- `llc gpu_object_lifetime.runtime.ll`
- See the assertion failure and stack dump (pasted below):

  Assertion failed: (!(Elmt.getFragmentOrDefault() == 
Next.getFragmentOrDefault())), function operator(), file 
AssignmentTrackingAnalysis.cpp, line 2020.
  PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ 
and include the crash backtrace.
  Stack dump:
  0.Program arguments: /Users/srj/llvm-17-install/bin/llc 
gpu_object_lifetime.runtime.ll
  1.Running pass 'Function Pass Manager' on module 
'gpu_object_lifetime.runtime.ll'.
  2.Running pass 'Assignment Tracking Analysis' on function 
'@halide_debug_to_file'
   #0 0x000106b92697 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) 
(/Users/srj/llvm-17-install/bin/llc+0x101fab697)
   #1 0x000106b90638 llvm::sys::RunSignalHandlers() 
(/Users/srj/llvm-17-install/bin/llc+0x101fa9638)
   #2 0x000106b92e40 SignalHandler(int) 
(/Users/srj/llvm-17-install/bin/llc+0x101fabe40)
   #3 0x7ff81c079dfd 
(/usr/lib/system/libsystem_platform.dylib+0x7ff800336dfd)
   #4 0x7ff7bb316720
   #5 0x7ff81bfafd24 (/usr/lib/system/libsystem_c.dylib+0x7ff80026cd24)
   #6 0x7ff81bfaf0cb (/usr/lib/system/libsystem_c.dylib+0x7ff80026c0cb)
   #7 0x0001076f1da3 void 
std::__1::__sort, 
llvm::DenseMapInfo, void>> const&, llvm::DenseMap, 1u>, llvm::DenseMapInfo, llvm::detail::DenseMapPair, 
1u>>>&, unsigned int&)::$_1&, llvm::DebugVariable*>(llvm::DebugVariable*, 
llvm::DebugVariable*, buildOverlapMapAndRecordDeclares(llvm::Function&, 
FunctionVarLocsBuilder*, llvm::DenseSet, 
llvm::DenseMapInfo, void>> const&, llvm::DenseMap, 1u>, llvm::DenseMapInfo, llvm::detail::DenseMapPair, 
1u>>>&, unsigned int&)::$_1&) (.cold.4) 
(/Users/srj/llvm-17-install/bin/llc+0x102b0ada3)
   #8 0x000105e3b47d void 
std::__1::__sort, 
llvm::DenseMapInfo, void>> const&, llvm::DenseMap, 1u>, llvm::DenseMapInfo, llvm::detail::DenseMapPair, 
1u>>>&, unsigned int&)::$_1&, llvm::DebugVariable*>(llvm::DebugVariable*, 
llvm::DebugVariable*, buildOverlapMapAndRecordDeclares(llvm::Function&, 
FunctionVarLocsBuilder*, llvm::DenseSet, 
llvm::DenseMapInfo, void>> const&, llvm::DenseMap, 1u>, llvm::DenseMapInfo, llvm::detail::DenseMapPair, 
1u>>>&, unsigned int&)::$_1&) (/Users/srj/llvm-17-install/bin/llc+0x10125447d)
   #9 0x000105e2ff8c (anonymous 
namespace)::AssignmentTrackingLowering::run(FunctionVarLocsBuilder*) 
(/Users/srj/llvm-17-install/bin/llc+0x101248f8c)
  #10 0x000105e2ce9e 
llvm::AssignmentTrackingAnalysis::runOnFunction(llvm::Function&) 
(/Users/srj/llvm-17-install/bin/llc+0x101245e9e)
  #11 0x00010649a082 llvm::FPPassManager::runOnFunction(llvm::Function&) 
(/Users/srj/llvm-17-install/bin/llc+0x1018b3082)
  #12 0x0001064a08a1 llvm::FPPassManager::runOnModule(llvm::Module&) 
(/Users/srj/llvm-17-install/bin/llc+0x1018b98a1)
  #13 0x00010649a687 llvm::legacy::PassManagerImpl::run(llvm::Module&) 
(/Users/srj/llvm-17-install/bin/llc+0x1018b3687)
  #14 0x000104beed44 compileModule(char**, llvm::LLVMContext&) 
(/Users/srj/llvm-17-install/bin/llc+0x17d44)
  #15 0x000104beca4f main (/Users/srj/llvm-17-install/bin/llc+0x15a4f)

F27211069: gpu_object_lifetime.runtime.ll.zip 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146987/new/

https://reviews.llvm.org/D146987

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

[PATCH] D146987: [Assignment Tracking] Enable by default

2023-04-21 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

In D146987#4287137 , @jmorse wrote:

> Ah, you're right, it's actually the same assertion message that @srj reported 
> which I totally skipped over. Currently I suspect that's due to different 
> std::sort implementations.

...`std::stable_sort()`?

(But more seriously, could we please revert all of this unless/until a fix is 
imminent? Our testing is dead in the water at the moment.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146987/new/

https://reviews.llvm.org/D146987

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

[PATCH] D146987: [Assignment Tracking] Enable by default

2023-04-21 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

In D146987#4287819 , @jmorse wrote:

> Should have been reverted in rG0ba922f60046 
>  earlier 
> today, are you still experiencing the same behaviour with that patch?

Ah, sorry, I didn't see that -- I'll pull and rebuild this morning to verify!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146987/new/

https://reviews.llvm.org/D146987

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

[PATCH] D146987: [Assignment Tracking] Enable by default

2023-04-21 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

In D146987#4287847 , @srj wrote:

> In D146987#4287819 , @jmorse wrote:
>
>> Should have been reverted in rG0ba922f60046 
>>  
>> earlier today, are you still experiencing the same behaviour with that patch?
>
> Ah, sorry, I didn't see that -- I'll pull and rebuild this morning to verify!

Looks good now, thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146987/new/

https://reviews.llvm.org/D146987

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

[PATCH] D100981: Delete le32/le64 targets

2021-04-22 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

Any chance that this could be (temporarily) reverted? This will break literally 
all Halide compilation; we're working on a fix on our side, but it would be 
nice to have a few days to be sure we have it right, and I suspect there's no 
urgency to removing this right now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100981/new/

https://reviews.llvm.org/D100981

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

[PATCH] D100981: Delete le32/le64 targets

2021-04-22 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

In D100981#2709205 , @MaskRay wrote:

> In D100981#2709044 , @srj wrote:
>
>> Any chance that this could be (temporarily) reverted? This will break 
>> literally all Halide compilation; we're working on a fix on our side, but it 
>> would be nice to have a few days to be sure we have it right, and I suspect 
>> there's no urgency to removing this right now.
>
> Will https://github.com/halide/Halide/pull/5934 take longer? If it does, I 
> can temporarily revert this.

Yeah, the first simple attempt in https://github.com/halide/Halide/pull/5934 
doesn't work (many things are broken). Temporarily reverting this would be a 
huge huge favor to us.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100981/new/

https://reviews.llvm.org/D100981

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

[PATCH] D100492: [OpenCL] Change OpenCL builtin version encoding

2021-04-26 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

This change appears to be the injection point for 
https://bugs.llvm.org/show_bug.cgi?id=50128 -- trying to build clang-tblgen 
after this change crashes VC2019 if targeting 32-bit Windows (see below). While 
this is likely an MSVC bug, I presume we still want to support crosscompiling 
for 32-bit  Windows?

  [2220/3953] Building CXX object 
tools\clang\utils\TableGen\CMakeFiles\clang-tblgen.dir\ClangOpenCLBuiltinEmitter.cpp.obj
  FAILED: 
tools/clang/utils/TableGen/CMakeFiles/clang-tblgen.dir/ClangOpenCLBuiltinEmitter.cpp.obj
 
  
C:\PROGRA~2\MICROS~2\2019\COMMUN~1\VC\Tools\MSVC\1428~1.293\bin\Hostx64\x86\cl.exe
  /nologo /TP -DCLANG_ROUND_TRIP_CC1_ARGS=ON -DUNICODE 
-D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS 
-D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_FILE_OFFSET_BITS=64 
-D_GNU_SOURCE -D_HAS_EXCEPTIONS=0 -D_LARGEFILE_SOURCE 
-D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE 
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
-Itools\clang\utils\TableGen 
-IC:\code\build_bot\worker\llvm-13-x86-32-windows\llvm-project\clang\utils\TableGen
 -IC:\code\build_bot\worker\llvm-13-x86-32-windows\llvm-project\clang\include 
-Itools\clang\include -Iinclude 
-IC:\code\build_bot\worker\llvm-13-x86-32-windows\llvm-project\llvm\include 
/DWIN32 /D_WINDOWS   /Zc:inline /Zc:__cplusplus /Zc:strictStrings /Oi 
/Zc:rvalueCast /bigobj /W4 -wd4141 -wd4146 -wd4244 -wd4267 -wd4291 -wd4351 
-wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4100 -wd4127 -wd4512 
-wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 
-wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4709 -wd4324 -w14062 
-we4238 /Gw /MD /O2 /Ob2  /EHs-c- /GR -UNDEBUG -std:c++14 /showIncludes 
/Fotools\clang\utils\TableGen\CMakeFiles\clang-tblgen.dir\ClangOpenCLBuiltinEmitter.cpp.obj
 /Fdtools\clang\utils\TableGen\CMakeFiles\clang-tblgen.dir\ /FS -c 
C:\code\build_bot\worker\llvm-13-x86-32-windows\llvm-project\clang\utils\TableGen\ClangOpenCLBuiltinEmitter.cpp
  
C:\code\build_bot\worker\llvm-13-x86-32-windows\llvm-project\clang\utils\TableGen\ClangOpenCLBuiltinEmitter.cpp(514)
 : fatal error C1001: Internal compiler error.
  (compiler file 'd:\A01\_work\3\s\src\vctools\Compiler\Utc\src\p2\main.c', 
line 212)
   To work around this problem, try simplifying or changing the program near 
the locations listed above.
  If possible please provide a repro here: 
https://developercommunity.visualstudio.com 
  Please choose the Technical Support command on the Visual C++ 
   Help menu, or open the Technical Support help file for more information
cl!RaiseException()+0x69
cl!RaiseException()+0x69
cl!CloseTypeServerPDB()+0x22ebf
cl!CloseTypeServerPDB()+0xedea9


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100492/new/

https://reviews.llvm.org/D100492

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

[PATCH] D100492: [OpenCL] Change OpenCL builtin version encoding

2021-04-26 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

From experimentation, it appears that just pulling the MinVersion and 
MaxVersion expressions from `BuiltinNameEmitter::EmitBuiltinTable` into 
separate statements will pacify MSVC, e.g.

  auto MinVersion = 
Overload.first->getValueAsDef("MinVersion")->getValueAsInt("ID");
  auto MaxVersion = 
Overload.first->getValueAsDef("MaxVersion")->getValueAsInt("ID");
  OS << "  { " << Overload.second << ", "
 << Overload.first->getValueAsListOfDefs("Signature").size() << ", "
 << (Overload.first->getValueAsBit("IsPure")) << ", "
 << (Overload.first->getValueAsBit("IsConst")) << ", "
 << (Overload.first->getValueAsBit("IsConv")) << ", "
 << FunctionExtensionIndex[ExtName] << ", "
 << MinVersion 
 << ", "
 << MaxVersion 
 << " },\n";


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100492/new/

https://reviews.llvm.org/D100492

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

[PATCH] D100492: [OpenCL] Change OpenCL builtin version encoding

2021-04-26 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

(This is clearly just a weird bug on MSVC's part; there's nothing about the 
code here that seems obviously unreasonable or complex. Working around compiler 
bugs is a thing, alas.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100492/new/

https://reviews.llvm.org/D100492

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

[PATCH] D100492: [OpenCL] Change OpenCL builtin version encoding

2021-04-26 Thread Steven Johnson via Phabricator via cfe-commits 
srj added a comment.

In D100492#2717235 , @svenvh wrote:

> I am happy to make/accept any adjustments to this patch to fix the particular 
> build.  I don't have access to MSVC though, would you be able to help figure 
> out what the problematic construct in my patch was?

See my comment above with minor code shuffling which seems to satisfy the 
compiler.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100492/new/

https://reviews.llvm.org/D100492

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