[PATCH] D96278: [clang][cli] Extract FileSystem and Migrator options parsing/generation

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

Thank you for the quick followup!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96278

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

[PATCH] D96278: [clang][cli] Extract FileSystem and Migrator options parsing/generation

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

This broke the windows bot: 
http://lab.llvm.org:8011/#/builders/86/builds/7540/steps/6/logs/stdio

  FAILED: 
tools/clang/lib/Frontend/CMakeFiles/obj.clangFrontend.dir/CompilerInvocation.cpp.obj
 
  
C:\PROGRA~2\MICROS~1\2017\COMMUN~1\VC\Tools\MSVC\1416~1.270\bin\Hostx64\x64\cl.exe
  /nologo /TP -DGTEST_HAS_RTTI=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE 
-D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS 
-D_GNU_SOURCE -D_HAS_EXCEPTIONS=0 -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\lib\Frontend 
-IC:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\clang\lib\Frontend
 
-IC:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\clang\include
 -Itools\clang\include -Iinclude 
-IC:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\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 /DNDEBUG/EHs-c- /GR- -std:c++14 /showIncludes 
/Fotools\clang\lib\Frontend\CMakeFiles\obj.clangFrontend.dir\CompilerInvocation.cpp.obj
 /Fdtools\clang\lib\Frontend\CMakeFiles\obj.clangFrontend.dir\ /FS -c 
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\clang\lib\Frontend\CompilerInvocation.cpp
  tools\clang\include\clang/Driver/Options.inc(6004): error C2065: 
'FileSystemOpts': undeclared identifier
  tools\clang\include\clang/Driver/Options.inc(6005): note: see reference to 
function template instantiation 'auto 
GenerateFileSystemArgsoperator 
()(const T &) const' being compiled


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96278

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

[PATCH] D95166: Disable rosegment for old Android versions.

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

It does feel kind of awkward to me that _this_ is the patch that ends up 
breaking the builds, versus something at the cmake level that says "you are 
explicitly unsupported".

Also it's unfortunate that this landed just hours before 12.0.0 branched. Had 
it landed slightly later, release users could have six months to plan for the 
change. You say that the next NDK compiler is based on ToT, in that case would 
it make sense to revert this on the 12.x branch, and add a warning to the 
12.0.0 release notes that lld will be assumed in 13?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95166

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

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

If I'm reading git correctly, the change is still present on the 12.x branch. 
Should it be reverted there too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91913

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

[PATCH] D95166: Disable rosegment for old Android versions.

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

Firefox has a build break from this change. In certain Android configurations 
we use bfd or gold. The statement in the commit message "Android only supports 
LLD" is news to me, could you point me to any references for this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95166

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

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

Our bots are green at b43c26d036dc 
. Many 
thanks @hvdijk for the quick response.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91913

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

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

On closer inspection (sorry, I'm juggling too many things this morning) it 
seems gcc does give a matching `int nArgs = 1` under `-std=c++17` mode.

So potentially the user code is wrong? One thing that puzzles me is why this 
only broke on our Windows bots. I am not clear if it has to do with the MS 
compatibility switches or whether coincidentally some platform ifdefs in the 
file prevented this line from being included on other platforms.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91913

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

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

The bot that reported this failure was using: `clang -cc1 -triple 
x86_64-pc-windows-msvc19.16.27038 -fms-extensions -fms-compatibility 
-fms-compatibility-version=19.16.27038 -std=c++17`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91913

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

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

We have a downstream build break due to this commit. One of our files has some 
convoluted arg-counting logic that now returns a different count, which does 
not match gcc: https://godbolt.org/z/W8caMr (Note: At time of writing, the 
clang trunk on godbolt doesn't yet have this change.)

IIUC the 12.0 release branch will be created tomorrow. If this isn't something 
that can be fixed quickly, please consider reverting until after the branch 
point.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91913

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

[PATCH] D92936: [Sema] Fix deleted function problem in implicitly movable test

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

I tried the updated patch against our build and it was successful. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92936

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

[PATCH] D92936: [Sema] Fix deleted function problem in implicitly movable test

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

Before the revert, our bots hit the following issue where we only error out 
when `-Wall` is given, so there's definitely something strange going on. Also 
happens with @Quuxplusone's suggested change applied.

  $ cat test.cpp
  template < class > class RefPtr {
  public:
~RefPtr();
template < typename d > RefPtr(d);
operator int() &;
operator int() && = delete;
  };
  class X;
  bool e() {
RefPtr< X > frame(0);
return frame;
  }
  
  $ clang -cc1 -std=c++17 test.cpp
  $ clang -cc1 -std=c++17 test.cpp -Wall
  test.cpp:11:10: error: conversion function from 'RefPtr' to 'bool' invokes 
a deleted function
return frame;
   ^
  test.cpp:6:3: note: 'operator int' has been explicitly marked deleted here
operator int() && = delete;
^
  1 error generated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92936

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

[PATCH] D91813: [PGO] verify BFI counts after loading profile data

[dmajor via Phabricator via cfe-commits]

dmajor added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:1948
+  // xur
+  fprintf(stderr, "hihi 2\n");
+  //  FuncAttrs.addAttribute(llvm::Attribute::Hot);

This looks like something for temporary local debugging, was it intentional to 
commit it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91813

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

[PATCH] D91747: [Clang] Add __STDCPP_THREADS__ to standard predefine macros

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

To connect the dots for posterity: the followup was landed in D92349 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91747

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

[PATCH] D91747: [Clang] Add __STDCPP_THREADS__ to standard predefine macros

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

In D91747#2412346 , @alanphipps wrote:

> Looks like _LIBCPP_HAS_NO_THREADS is being set for libcxxabi, and the build 
> now fails with this change:
>
> llvm-project/libcxxabi/../libcxx/include/__config:1172:2: error: 
> _LIBCPP_HAS_NO_THREADS cannot be set when __STDCPP_THREADS__ is set

Indeed: LIBCXX_ENABLE_THREADS is off by default.

@zequanwu, what's your take on this... which part of the system needs to adapt?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91747

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

[PATCH] D91330: [clangd] Ensure we test for compatibility of serialized index format

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

In D91330#2394415 , @kadircet wrote:

> Looks like it is already green at 
> http://lab.llvm.org:8011/#/builders/109/builds/2693

Yep, I'm seeing the same, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91330

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

[PATCH] D91330: [clangd] Ensure we test for compatibility of serialized index format

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

This broke the bots: http://lab.llvm.org:8011/#/builders/109/builds/2682

File 
"/b/1/clang-x86_64-debian-fast/llvm.obj/tools/clang/tools/extra/clangd/test/lit.site.cfg.py",
 line 35
  config.have_zlib =
^
  SyntaxError: invalid syntax


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91330

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

[PATCH] D88609: Use uint64_t for branch weights instead of uint32_t

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

After 10f2a0d662d8 
 our 
self-host PGO builds are hitting:
llvm::BranchProbability::getBranchProbability(uint64_t, uint64_t): Assertion 
`Numerator <= Denominator && "Probability cannot be bigger than 1!"' failed.

https://treeherder.mozilla.org/logviewer?job_id=320324631=try=52597
 has the log as well as the .cpp/.sh reproducers, but I assume you'd also need 
the merged.profdata file, and our bots haven't been taught to save those. I can 
try to add that ability next week if needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88609

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

[PATCH] D87528: Enable '#pragma STDC FENV_ACCESS' in frontend

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

After this commit our bots have a failure on the wasm target, could you please 
take a look?

test.c:

  double a, b;
  c() {
  #pragma STDC FENV_ACCESS ON
b == a;
  }

Run `clang -cc1 -triple wasm32-unknown-wasi -emit-obj test.c` (clang needs to 
be build with `-DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD=WebAssembly`)

Result:

  clang: 
/home/vm/src/llvm-project/llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:3764: 
bool (anonymous namespace)::SelectionDAGLegalize::ExpandNode(llvm::SDNode *): 
Assertion `!IsStrict && "Don't know how to expand for strict nodes."' failed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

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

[PATCH] D88005: [clang] [MinGW] Add an implicit .exe suffix even when crosscompiling

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

This change broke the configure step of Firefox mingw builds.

The build bot won't give me the full details, so I'll need to set up a local 
repro next week if needed, but my hunch is that we have some test like `$CC 
$CFLAGS conftest.c -o conftest` and then check for the existence of `conftest`.

Is that type of pattern an acceptable casualty of this change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88005

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

[PATCH] D83004: [UpdateCCTestChecks] Include generated functions if asked

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

The expensive-checks bots have been red for several days. Could you please take 
a look or revert?

riscv_generated_funcs.test:

  *** Bad machine code: MBB exits via unconditional fall-through but ends with 
a barrier instruction! ***
  - function:check_boundaries
  - basic block: %bb.4  (0x5653e22035b8)
  LLVM ERROR: Found 1 machine code errors.

http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-ubuntu/builds/9171/steps/test-check-all/logs/FAIL%3A%20LLVM%3A%3Ariscv_generated_funcs.test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83004

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

[PATCH] D87163: [DSE] Switch to MemorySSA-backed DSE by default.

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

I threw as many tests as I could find at 9d172c8e9c84 
, and I 
don't see any regressions compared to its parent revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87163

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

[PATCH] D87917: [Sema] Handle objc_super special lookup when checking builtin compatibility

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

Thanks, this fixes the specific translation unit I was looking at today. It's 
going to take some time before I have full build results but I don't expect any 
problems.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87917

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

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

This commit broke Firefox builds on Mac with an error in the SDK headers. Could 
you please revert if a fix is not readily available?

Reproducer:

  struct objc_super {};
  extern "C" id objc_msgSendSuper(struct objc_super *super, SEL op, ...);
  extern "C" void objc_msgSendSuper_stret(struct objc_super *super, SEL op, 
...);

Result:

  $ clang -c test.mm
  test.mm:3:48: error: reference to 'objc_super' is ambiguous
  extern "C" void objc_msgSendSuper_stret(struct objc_super *super, SEL op, 
...);
 ^
  test.mm:1:8: note: candidate found by name lookup is 'objc_super'
  struct objc_super {};
 ^
  note: candidate found by name lookup is 'objc_super'
  1 error generated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

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

[PATCH] D87163: [DSE] Switch to MemorySSA-backed DSE by default.

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

Thanks for the heads up. I'll schedule a more intensive test run over the 
weekend while there's less demand for machines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87163

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

[PATCH] D87701: Do not apply calling conventions to MSVC entry points

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

This broke Firefox builds too, in one of our helper binaries that uses a 
`wWinMain`, although I'm having trouble writing a minimal reproducer for it. 
Simply making a barebones `wWinMain` program doesn't hit the error.

If the patch re-lands, please cc me and I'll re-test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87701

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

[PATCH] D87163: [DSE] Switch to MemorySSA-backed DSE by default.

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

In D87163#2275896 , @asbirlea wrote:

> I checked in a fix in https://reviews.llvm.org/rGfc8200633122, but I haven't 
> yet verified it addresses all the failures reported.

Thanks, I've confirmed that this fixes our tests in their original form (not 
reduced).

Our day-to-day testing of LLVM trunk is limited though, maybe one-tenth of our 
full suite. Since this code has some risks, I could start a full run to throw 
more testing at it. But that takes more machine capacity and human effort to 
remove flaky failures, so I'd prefer to wait until the odds are good that there 
won't be further changes. Let me know when you think it's ready.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87163

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

[PATCH] D87163: [DSE] Switch to MemorySSA-backed DSE by default.

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

Reduced a bit more: https://godbolt.org/z/j59evK (C++) and 
https://godbolt.org/z/8xG688 (IR) -- the store at line 43 of `while.end` has 
been removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87163

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

[PATCH] D87163: [DSE] Switch to MemorySSA-backed DSE by default.

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

Here is a reduced-ish repro, I can keep poking at it but maybe this is 
sufficient for you to notice something: https://godbolt.org/z/endf6d.

Center pane is old DSE, right pane is new DSE. I highlighted the problem area 
with a nop at line 56 of the source, and line 59 of each disassembly. The array 
writes following the nop have been eliminated, even though they will be needed 
on a future iteration of the for loop.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87163

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

[PATCH] D87163: [DSE] Switch to MemorySSA-backed DSE by default.

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

In D87163#2272105 , @fhahn wrote:

> I also pushed a fix for a MemorySSA issue that was exposed through DSE + 
> MemorySSA in c4f1b3144184 
> . This 
> might also fix (some) of the failures you are seeing.

Same failures with c4f1b3144184 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87163

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

[PATCH] D87163: [DSE] Switch to MemorySSA-backed DSE by default.

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

In D87163#2270871 , @fhahn wrote:

> In D87163#2270444 , @thakis wrote:
>
>> Heads-up: We're seeing fairly widespread test failures with this in Chromium 
>> that go away when we force this flag off. It's possible that it's due to 
>> only a small number of issues and they might be previously-benign UB (no 
>> idea), but I figured I'd give an early note. We'll send an update once we've 
>> reduced a failing test.
>
> Thanks for the heads-up. I just pushed another fix for the issue @uabelho. 
> I'd guess at least some of the failures you are seeing should be related. If 
> not, I think it makes sense to revert this once we have the reproducers and 
> can work in ironing out the remaining issues.

We're also seeing test failures in Firefox that are not fixed by f715d81c9df3 
. I'll 
attempt to reduce but it may take some time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87163

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

[PATCH] D85545: [Diagnostics] Diagnose missing comma in string array initialization

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

In the Firefox repo this warning is firing on a number of strings that were 
broken up by clang-format (or humans) for line length, for example 
https://searchfox.org/mozilla-central/rev/ab81b8552f4aa9696a2524f97fdfeb59d4dc31c1/security/certverifier/ExtendedValidation.cpp#176-178
 or 
https://searchfox.org/mozilla-central/rev/ab81b8552f4aa9696a2524f97fdfeb59d4dc31c1/xpcom/tests/gtest/TestEscape.cpp#103-104
 or 
https://searchfox.org/mozilla-central/rev/ab81b8552f4aa9696a2524f97fdfeb59d4dc31c1/js/src/jsapi-tests/testXDR.cpp#115.

Do you consider these to be false positives in your view? Should there be some 
exception for line length, perhaps?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85545

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

[PATCH] D82562: Implement AVX ABI Warning/error

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

@erichkeane, could you help me understand what is the action item of these 
warnings?

In Firefox we don't require AVX so our compilations generally don't enable the 
feature. (A very small number of files do come with AVX versions, mostly in 
imported media codecs, but they are reasonably well-isolated from other code.) 
It's not clear to me what the warning wants me to do. I can't enable AVX across 
the board, and I can't change all the code that uses large types as parameters. 
I feel like the only thing to do is silence the warning since there are enough 
instances that people will complain about log spam. Have I misunderstood the 
situation?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82562

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

[PATCH] D83494: [libFuzzer] Link libFuzzer's own interceptors when other compiler runtimes are not linked.

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

After this commit, several of our builds are failing with 
`FuzzerInterceptors.cpp:30:10: fatal error: 'sanitizer/common_interface_defs.h' 
file not found`. This is odd because the file certainly seems like it exists. 
Is there a missing include path somewhere, perhaps?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83494



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

[PATCH] D65543: [Windows] Autolink with basenames and add libdir to libpath

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

In D65543#2123200 , @dmajor wrote:

> In that case, more specifically, the problem I'm seeing is that these flags 
> are not making it through to a small handful of files: LLVMHello.dll, 
> PrintFunctionNames.dll, AnnotateFunctions.dll, Attribute.dll. Let me dig in a 
> little deeper...


Patch in D82888 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65543



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

[PATCH] D65543: [Windows] Autolink with basenames and add libdir to libpath

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

In D65543#2122942 , @dmajor wrote:

> After this change, PGO builds of clang on Windows can no longer find 
> `clang_rt.profile-x86_64.lib`. Is there some cmake magic that could 
> potentially make this seamless? Or is it expected that the burden is on the 
> user to set up the paths manually during the `LLVM_BUILD_INSTRUMENTED` phase?


Err, it looks like this commit already has exactly such cmake magic!

In that case, more specifically, the problem I'm seeing is that these flags are 
not making it through to a small handful of files: LLVMHello.dll, 
PrintFunctionNames.dll, AnnotateFunctions.dll, Attribute.dll. Let me dig in a 
little deeper...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65543



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

[PATCH] D65543: [Windows] Autolink with basenames and add libdir to libpath

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

After this change, PGO builds of clang on Windows can no longer find 
`clang_rt.profile-x86_64.lib`. Is there some cmake magic that could potentially 
make this seamless? Or is it expected that the burden is on the user to set up 
the paths manually during the `LLVM_BUILD_INSTRUMENTED` phase?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65543



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

[PATCH] D74784: [driver][darwin] Don't use -platform_version flag by default

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

Thanks! I'm still working on getting commit access, would one of you be able to 
submit for me?


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

https://reviews.llvm.org/D74784



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

[PATCH] D74784: [driver][darwin] Don't use -platform_version flag by default

[dmajor via Phabricator via cfe-commits]

dmajor updated this revision to Diff 246771.
dmajor edited the summary of this revision.
dmajor added a comment.

Updated the tests: three cases for {default, old, new} linkers in the 
platform-version tests; left alone the tests not specifically targeting this 
path.


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

https://reviews.llvm.org/D74784

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-ld-platform-version-ios.c
  clang/test/Driver/darwin-ld-platform-version-macos.c
  clang/test/Driver/darwin-ld-platform-version-tvos.c
  clang/test/Driver/darwin-ld-platform-version-watchos.c


Index: clang/test/Driver/darwin-ld-platform-version-watchos.c
===
--- clang/test/Driver/darwin-ld-platform-version-watchos.c
+++ clang/test/Driver/darwin-ld-platform-version-watchos.c
@@ -1,9 +1,14 @@
 // RUN: touch %t.o
 
+// RUN: %clang -target arm64_32-apple-watchos5.2 -isysroot 
%S/Inputs/WatchOS6.0.sdk -mlinker-version=0 -### %t.o 2>&1 \
+// RUN:   | FileCheck --check-prefix=LINKER-OLD %s
+// RUN: %clang -target arm64_32-apple-watchos5.2 -isysroot 
%S/Inputs/WatchOS6.0.sdk -mlinker-version=400 -### %t.o 2>&1 \
+// RUN:   | FileCheck --check-prefix=LINKER-OLD %s
 // RUN: %clang -target arm64_32-apple-watchos5.2 -isysroot 
%S/Inputs/WatchOS6.0.sdk -mlinker-version=520 -### %t.o 2>&1 \
-// RUN:   | FileCheck %s
+// RUN:   | FileCheck --check-prefix=LINKER-NEW %s
 // RUN: %clang -target x86_64-apple-watchos6-simulator -isysroot 
%S/Inputs/WatchOS6.0.sdk -mlinker-version=520 -### %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=SIMUL %s
 
-// CHECK: "-platform_version" "watchos" "5.2.0" "6.0.0"
+// LINKER-OLD: "-watchos_version_min" "5.2.0"
+// LINKER-NEW: "-platform_version" "watchos" "5.2.0" "6.0.0"
 // SIMUL: "-platform_version" "watchos-simulator" "6.0.0" "6.0.0"
Index: clang/test/Driver/darwin-ld-platform-version-tvos.c
===
--- clang/test/Driver/darwin-ld-platform-version-tvos.c
+++ clang/test/Driver/darwin-ld-platform-version-tvos.c
@@ -1,9 +1,14 @@
 // RUN: touch %t.o
 
+// RUN: %clang -target arm64-apple-tvos12.3 -isysroot 
%S/Inputs/iPhoneOS13.0.sdk -mlinker-version=0 -### %t.o 2>&1 \
+// RUN:   | FileCheck --check-prefix=LINKER-OLD %s
+// RUN: %clang -target arm64-apple-tvos12.3 -isysroot 
%S/Inputs/iPhoneOS13.0.sdk -mlinker-version=400 -### %t.o 2>&1 \
+// RUN:   | FileCheck --check-prefix=LINKER-OLD %s
 // RUN: %clang -target arm64-apple-tvos12.3 -isysroot 
%S/Inputs/iPhoneOS13.0.sdk -mlinker-version=520 -### %t.o 2>&1 \
-// RUN:   | FileCheck %s
+// RUN:   | FileCheck --check-prefix=LINKER-NEW %s
 // RUN: %clang -target x86_64-apple-tvos13-simulator -isysroot 
%S/Inputs/iPhoneOS13.0.sdk -mlinker-version=520 -### %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=SIMUL %s
 
-// CHECK: "-platform_version" "tvos" "12.3.0" "13.0"
+// LINKER-OLD: "-tvos_version_min" "12.3.0"
+// LINKER-NEW: "-platform_version" "tvos" "12.3.0" "13.0"
 // SIMUL: "-platform_version" "tvos-simulator" "13.0.0" "13.0"
Index: clang/test/Driver/darwin-ld-platform-version-macos.c
===
--- clang/test/Driver/darwin-ld-platform-version-macos.c
+++ clang/test/Driver/darwin-ld-platform-version-macos.c
@@ -1,11 +1,14 @@
 // RUN: touch %t.o
 
 // RUN: %clang -target x86_64-apple-macos10.13 -isysroot 
%S/Inputs/MacOSX10.14.sdk -mlinker-version=0 -### %t.o 2>&1 \
-// RUN:   | FileCheck %s
+// RUN:   | FileCheck --check-prefix=LINKER-OLD %s
+// RUN: %clang -target x86_64-apple-macos10.13 -isysroot 
%S/Inputs/MacOSX10.14.sdk -mlinker-version=400 -### %t.o 2>&1 \
+// RUN:   | FileCheck --check-prefix=LINKER-OLD %s
 // RUN: env SDKROOT=%S/Inputs/MacOSX10.14.sdk %clang -target 
x86_64-apple-macos10.13.0.1 -mlinker-version=520 -### %t.o 2>&1 \
-// RUN:   | FileCheck %s
+// RUN:   | FileCheck --check-prefix=LINKER-NEW %s
 
-// CHECK: "-platform_version" "macos" "10.13.0" "10.14"
+// LINKER-OLD: "-macosx_version_min" "10.13.0"
+// LINKER-NEW: "-platform_version" "macos" "10.13.0" "10.14"
 
 // RUN: %clang -target x86_64-apple-macos10.13  -mlinker-version=520 -### %t.o 
2>&1 \
 // RUN:   | FileCheck --check-prefix=NOSDK %s
Index: clang/test/Driver/darwin-ld-platform-version-ios.c
===
--- clang/test/Driver/darwin-ld-platform-version-ios.c
+++ clang/test/Driver/darwin-ld-platform-version-ios.c
@@ -1,9 +1,14 @@
 // RUN: touch %t.o
 
+// RUN: %clang -target arm64-apple-ios12.3 -isysroot 
%S/Inputs/iPhoneOS13.0.sdk -mlinker-version=0 -### %t.o 2>&1 \
+// RUN:   | FileCheck --check-prefix=LINKER-OLD %s
+// RUN: %clang -target arm64-apple-ios12.3 -isysroot 
%S/Inputs/iPhoneOS13.0.sdk -mlinker-version=400 -### %t.o 2>&1 \
+// RUN:   | FileCheck --check-prefix=LINKER-OLD %s
 // RUN: %clang -target arm64-apple-ios12.3 -isysroot 
%S/Inputs/iPhoneOS13.0.sdk 

[PATCH] D74784: [driver][darwin] Don't use -platform_version flag by default

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

@steven_wu, ping, could you clarify about the tests please?

>> You should definitely add test for this change. The fact that you change all 
>> `-mlinker-version=400` to `-mlinker-version=0` but not change any CHECK 
>> lines means the change is definitely not tested :)
> 
> Can you elaborate on what you would like to see tested? I thought I _was_ 
> updating the tests to match the code. The `-mlinker-version=0` lines check 
> that we now use the old behavior by default, and the existing 
> `-mlinker-version=520` lines in e.g. `darwin-ld-platform-version-macos.c` 
> test that we still use the new behavior when requested.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74784



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

[PATCH] D74784: [driver][darwin] Don't use -platform_version flag by default

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

In D74784#1882974 , @steven_wu wrote:

> I forgot if there is reason to use the option by default at all time (I did 
> ask that in the previous review but Alex might have given more context 
> offline).


I would really like to hear from @arphaman, but they haven't responded to 
anything since I filed PR44813 nearly two weeks ago. Do you have any means to 
contact them?

> You should definitely add test for this change. The fact that you change all 
> `-mlinker-version=400` to `-mlinker-version=0` but not change any CHECK lines 
> means the change is definitely not tested :)

Can you elaborate on what you would like to see tested? I thought I _was_ 
updating the tests to match the code. The `-mlinker-version=0` lines check that 
we now use the old behavior by default, and the existing `-mlinker-version=520` 
lines in e.g. `darwin-ld-platform-version-macos.c` test that we still use the 
new behavior when requested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74784



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

[PATCH] D74784: [driver][darwin] Don't use -platform_version flag by default

[dmajor via Phabricator via cfe-commits]

dmajor created this revision.
dmajor added reviewers: arphaman, steven_wu, dexonsmith.
dmajor added projects: clang, LLVM.
Herald added a subscriber: cfe-commits.

(Note, I don't currently have commit access.)

The code in llvmorg-10-init-12188-g25ce33a6e4f is a breaking change for users 
of older linkers who don't pass a version parameter, which prevents a drop-in 
clang upgrade. Old tools can't know about what future tools will do, so as a 
general principle the burden should be new tools to be compatible by default. 
Also, for comparison, none of the other tests of `Version` within `AddLinkArgs` 
add any new behaviors unless the version is explicitly specified. Therefore, 
this patch changes the `-platform_version` behavior from opt-out to opt-in.

In light of the test fixup in llvmorg-10-init-12213-gbe88a20c900, I've used 
`-mlinker-version=0` instead of doing a straight revert of the additions of 
`-mlinker-version=400`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74784

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-infer-simulator-sdkroot.c
  clang/test/Driver/darwin-ld-platform-version-macos.c
  clang/test/Driver/darwin-ld.c
  clang/test/Driver/darwin-sdkroot.c
  clang/test/Driver/target-triple-deployment.c

Index: clang/test/Driver/target-triple-deployment.c
===
--- clang/test/Driver/target-triple-deployment.c
+++ clang/test/Driver/target-triple-deployment.c
@@ -1,14 +1,14 @@
 // RUN: touch %t.o
-// RUN: %clang -target x86_64-apple-macosx10.4 -mlinker-version=400 -### %t.o 2> %t.log
-// RUN: %clang -target x86_64-apple-darwin9 -mlinker-version=400 -### %t.o 2>> %t.log
-// RUN: %clang -target x86_64-apple-macosx10.7 -mlinker-version=400 -### %t.o 2>> %t.log
+// RUN: %clang -target x86_64-apple-macosx10.4 -mlinker-version=0 -### %t.o 2> %t.log
+// RUN: %clang -target x86_64-apple-darwin9 -mlinker-version=0 -### %t.o 2>> %t.log
+// RUN: %clang -target x86_64-apple-macosx10.7 -mlinker-version=0 -### %t.o 2>> %t.log
 //
-// RUN: %clang -target armv7-apple-ios -mlinker-version=400 -### %t.o 2>> %t.log
-// RUN: %clang -target armv7-apple-ios0.0 -mlinker-version=400 -### %t.o 2>> %t.log
-// RUN: %clang -target armv7-apple-ios1.2.3 -mlinker-version=400 -### %t.o 2>> %t.log
-// RUN: %clang -target armv7-apple-ios5.0 -mlinker-version=400 -### %t.o 2>> %t.log
-// RUN: %clang -target armv7-apple-ios7.0 -mlinker-version=400 -### %t.o 2>> %t.log
-// RUN: %clang -target arm64-apple-ios -mlinker-version=400 -### %t.o 2>> %t.log
+// RUN: %clang -target armv7-apple-ios -mlinker-version=0 -### %t.o 2>> %t.log
+// RUN: %clang -target armv7-apple-ios0.0 -mlinker-version=0 -### %t.o 2>> %t.log
+// RUN: %clang -target armv7-apple-ios1.2.3 -mlinker-version=0 -### %t.o 2>> %t.log
+// RUN: %clang -target armv7-apple-ios5.0 -mlinker-version=0 -### %t.o 2>> %t.log
+// RUN: %clang -target armv7-apple-ios7.0 -mlinker-version=0 -### %t.o 2>> %t.log
+// RUN: %clang -target arm64-apple-ios -mlinker-version=0 -### %t.o 2>> %t.log
 //
 // RUN: FileCheck %s < %t.log
 
Index: clang/test/Driver/darwin-sdkroot.c
===
--- clang/test/Driver/darwin-sdkroot.c
+++ clang/test/Driver/darwin-sdkroot.c
@@ -43,7 +43,7 @@
 //
 // RUN: rm -rf %t/SDKs/iPhoneOS8.0.0.sdk
 // RUN: mkdir -p %t/SDKs/iPhoneOS8.0.0.sdk
-// RUN: env SDKROOT=%t/SDKs/iPhoneOS8.0.0.sdk %clang -target arm64-apple-darwin -mlinker-version=400 --sysroot="" %s -### 2>&1 \
+// RUN: env SDKROOT=%t/SDKs/iPhoneOS8.0.0.sdk %clang -target arm64-apple-darwin -mlinker-version=0 --sysroot="" %s -### 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-IPHONE %s
 //
 // CHECK-IPHONE: clang
@@ -55,7 +55,7 @@
 //
 // RUN: rm -rf %t/SDKs/iPhoneSimulator8.0.sdk
 // RUN: mkdir -p %t/SDKs/iPhoneSimulator8.0.sdk
-// RUN: env SDKROOT=%t/SDKs/iPhoneSimulator8.0.sdk %clang -target x86_64-apple-darwin -mlinker-version=400 --sysroot="" %s -### 2>&1 \
+// RUN: env SDKROOT=%t/SDKs/iPhoneSimulator8.0.sdk %clang -target x86_64-apple-darwin -mlinker-version=0 --sysroot="" %s -### 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-SIMULATOR %s
 //
 // CHECK-SIMULATOR: clang
@@ -66,7 +66,7 @@
 //
 // RUN: rm -rf %t/SDKs/MacOSX10.10.0.sdk
 // RUN: mkdir -p %t/SDKs/MacOSX10.10.0.sdk
-// RUN: env SDKROOT=%t/SDKs/MacOSX10.10.0.sdk %clang -target x86_64-apple-darwin -mlinker-version=400 --sysroot="" %s -### 2>&1 \
+// RUN: env SDKROOT=%t/SDKs/MacOSX10.10.0.sdk %clang -target x86_64-apple-darwin -mlinker-version=0 --sysroot="" %s -### 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-MACOSX %s
 //
 // CHECK-MACOSX: clang
Index: clang/test/Driver/darwin-ld.c
===
--- clang/test/Driver/darwin-ld.c
+++ clang/test/Driver/darwin-ld.c
@@ -11,9 +11,9 @@
 
 // Check linker changes that came with new linkedit format.
 // RUN: touch %t.o
-// RUN: %clang -target i386-apple-darwin9 

[PATCH] D71579: [driver][darwin] Pass -platform_version flag to the linker instead of the -_version_min flag

[dmajor via Phabricator via cfe-commits]

dmajor added inline comments.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:339
   // Add the deployment target.
-  MachOTC.addMinVersionArgs(Args, CmdArgs);
+  if (!Version[0] || Version[0] >= 520)
+MachOTC.addPlatformVersionArgs(Args, CmdArgs);

arphaman wrote:
> steven_wu wrote:
> > Why version '0' should go through the new path?
> Version `0` indicates that `-mlinker-version=` wasn't specified. We want to 
> use the new flag by default.
Hi. Just for anyone who isn't on the bugzilla thread: in 
https://bugs.llvm.org/show_bug.cgi?id=44813 I'd like to reconsider this 
decision about using the flag by default. It is a breaking change and is 
inconsistent with the other treatments of `Version` in this function. What do 
you all think? Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71579



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

[PATCH] D67678: PR17164: Change clang's default behavior from -flax-vector-conversions=all to -flax-vector-conversions=integer.

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

I noticed that this was merged to the 10.0 release branch. Should the merge be 
reverted while the dust settles on the trunk implementation?

FWIW this change breaks code in skia, as used in both Firefox and Chromium. I 
see that crbug.com/1042470 added the flag as a bandaid, and we'll probably have 
to do the same in Firefox. It's a bit frustrating to see a breaking change like 
this pop up late in the release cycle.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67678



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

[PATCH] D72167: Add support for __declspec(guard(nocf))

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

I've confirmed that the current patch fixes our CFG failures. Thanks again!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72167



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

[PATCH] D72167: Add support for __declspec(guard(nocf))

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

Is the current patch an interdiff? It would be helpful to have the diff against 
the master repo; Phabricator can take care of showing interdiffs if necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72167



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

[PATCH] D72167: Add support for __declspec(guard(nocf))

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

Thanks for doing this!

When I applied this patch and added `__declspec(guard(nocf))` to the function 
that was giving us trouble 
,
 I still crashed with a CFG failure in the caller, since the `nocf` function 
was inlined. When I additionally marked that function as `noinline`, then the 
CFG checks were omitted, as desired. Is this behavior with respect to inlining 
expected?

You may want to mention "PR44096" in the commit message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72167



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

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

In D65761#1772031 , @rnk wrote:

> I think `-fcf-protection` and `__attribute__((nocf_check))` have to do with 
> CET and Intel's endbranch instruction or what have you. Similar goals, 
> different implementation. I think at this point it's "patches welcome". =S


Well, this patch specifically has code (even a test) that `nocf_check` prevents 
CFG, so it looks like the intent was there, it's just that there isn't a way to 
get that attribute onto functions from cpp in a CFG/non-CET world.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65761



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

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

Are there any plans to implement `__declspec(guard(nocf))` or an equivalent 
mechanism? `__attribute__((nocf_check))` doesn't do anything without the 
-fcf-protection flag. (https://bugs.llvm.org/show_bug.cgi?id=44096)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65761



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

[PATCH] D68351: [profile] Add a mode to continuously sync counter updates to a file

[dmajor via Phabricator via cfe-commits]

dmajor added inline comments.



Comment at: compiler-rt/lib/profile/InstrProfilingFile.c:377
+#if defined(__Fuchsia__) || defined(_WIN32)
+  PROF_ERR("%s\n", "Continuous mode not yet supported on Fuchsia or Windows.");
+#else // defined(__Fuchsia__) || defined(_WIN32)

@vsk Doesn't this unconditionally break profiling on Fuschia/Windows? Should 
there at least be a check for `__llvm_profile_is_continuous_mode_enabled()` 
before erroring out? I'm currently testing my project's Windows build with 
trunk, and it's failing here even though we didn't enable continuous mode.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68351



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

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

Hi, I filed https://bugs.llvm.org/show_bug.cgi?id=44049 for some strange 
crashes that we're seeing because the CFG code overwrites the lower byte of 
function pointers before jumping to them. (Commenting separately here because I 
was unable to CC @ajpaverd in Bugzilla)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65761



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