Page MenuHomePhabricator

alexfh (Alexander Kornienko)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 10 2012, 10:35 AM (559 w, 4 d)

Recent Activity

Tue, Mar 28

alexfh added a comment to rG1e0709167f5e: Revert "[Modules] Remove unnecessary check when generating name lookup table in….

The relanded commit caused the same problem to reappear. Reverted it in 3048c9e15498161572483e3d4b81c9c37bf2770f. Unfortunately we still don't have a reproducer. My apologies.

Tue, Mar 28, 6:16 PM · Restricted Project, Restricted Project
alexfh added a comment to D144568: [libc++][NFC] Refactor the __enable_ifs in <string>.

This commit increases compilation time by ~15% and increases recursive template instantiation depth (thus, requiring adjusting -ftemplate-depth=2048) in some of our internal test code. Admittedly, it's using templates in a sort of a compiler-load-test-like way, but this change may affect compilation time for real code as well (it's just not likely to hit the default -ftemplate-depth=1024 limit and I don't have a good idea of how to find this code with reasonable effort).

I figured out. The test is really a corner case and it's not representative of real code. It artificially creates a chain of 1024 recursive template instantiations and the deepest level started requiring one more template instantiation for std::string after this patch. This is definitely not a problem elsewhere.

Tue, Mar 28, 5:04 PM · Restricted Project, Restricted Project
alexfh added a comment to D144568: [libc++][NFC] Refactor the __enable_ifs in <string>.

This commit increases compilation time by ~15% and increases recursive template instantiation depth (thus, requiring adjusting -ftemplate-depth=2048) in some of our internal test code. Admittedly, it's using templates in a sort of a compiler-load-test-like way, but this change may affect compilation time for real code as well (it's just not likely to hit the default -ftemplate-depth=1024 limit and I don't have a good idea of how to find this code with reasonable effort).

Was this sort of a compiler time regression anticipated with for this change?

I didn't expect really any change w.r.t. compile time. I also don't see how it would affect the amount of recursive template instantiations, since it doesn't add any layers. It just moves the instantiations into the type list. Do you have any specific cases where it changes?

Tue, Mar 28, 2:44 PM · Restricted Project, Restricted Project
alexfh added a comment to D144568: [libc++][NFC] Refactor the __enable_ifs in <string>.

This commit increases compilation time by ~15% and increases recursive template instantiation depth (thus, requiring adjusting -ftemplate-depth=2048) in some of our internal test code. Admittedly, it's using templates in a sort of a compiler-load-test-like way, but this change may affect compilation time for real code as well (it's just not likely to hit the default -ftemplate-depth=1024 limit and I don't have a good idea of how to find this code with reasonable effort).

Tue, Mar 28, 10:21 AM · Restricted Project, Restricted Project

Thu, Mar 16

alexfh added a comment to D144262: [libc++][ranges] Implement LWG-3865 Sorting a range of pairs.

This is a known issue, see this bug report https://bugs.mysql.com/bug.php?id=110254

Thu, Mar 16, 11:23 AM · Restricted Project, Restricted Project
alexfh added a comment to D144262: [libc++][ranges] Implement LWG-3865 Sorting a range of pairs.

The following snippet (reduced from a version of mysql) compiles well before this commit and fails to compile after it (https://gcc.godbolt.org/z/zrGbbbWb5):

Thu, Mar 16, 9:40 AM · Restricted Project, Restricted Project

Feb 23 2023

alexfh added a reverting change for rG8498ba6c2860: [clang] Add the check of membership for the issue #58674 and improve the lookup…: rG1812e13a3d6b: Revert "[clang] Add the check of membership for the issue #58674 and improve….
Feb 23 2023, 8:31 AM · Restricted Project, Restricted Project
alexfh committed rG1812e13a3d6b: Revert "[clang] Add the check of membership for the issue #58674 and improve… (authored by alexfh).
Revert "[clang] Add the check of membership for the issue #58674 and improve…
Feb 23 2023, 8:31 AM · Restricted Project, Restricted Project
alexfh added a reverting change for D143840: [clang] Add the check of membership for the issue #58674 and improve the lookup process: rG1812e13a3d6b: Revert "[clang] Add the check of membership for the issue #58674 and improve….
Feb 23 2023, 8:31 AM · Restricted Project, Restricted Project
alexfh added a comment to D143840: [clang] Add the check of membership for the issue #58674 and improve the lookup process.

Heads up: We are experiencing a series of clang crashes because of this commit.

What we already have so far:

Unhandled DeclRefExpr
UNREACHABLE executed at clang/lib/CodeGen/CGExpr.cpp:2958!

We are now working on a reproducer.

Here is our reproducer:

struct a {
  enum { b };
};
struct c {
  void *d;
};
struct e {
  void f(void *);
};
template <typename g> struct h : g {
  h(short, int);
  virtual void i();
};
template <typename g> void h<g>::i() { e().f(c::d); }
struct j : h<c> {
  j();
};
int k;
j::j() : h(a::b, k) {}
Feb 23 2023, 8:05 AM · Restricted Project, Restricted Project

Feb 15 2023

alexfh added a comment to D135462: [SelectionDAG] Do not second-guess alignment for alloca.

The patch is merged (again). Please let me know if it causes any regressions for in-tree targets or tests.

An early heads up: we started seeing some test failures after this commit, but I can't yet tell whether the commit or the code is at fault.

The only data point so far is that failures are happening on AArch64, which is alignment-sensitive.

Feb 15 2023, 10:41 AM · Restricted Project, Restricted Project

Feb 14 2023

alexfh added a comment to D135462: [SelectionDAG] Do not second-guess alignment for alloca.

The patch is merged (again). Please let me know if it causes any regressions for in-tree targets or tests.

An early heads up: we started seeing some test failures after this commit, but I can't yet tell whether the commit or the code is at fault.

Feb 14 2023, 5:48 AM · Restricted Project, Restricted Project
alexfh added a comment to D135462: [SelectionDAG] Do not second-guess alignment for alloca.

The patch is merged (again). Please let me know if it causes any regressions for in-tree targets or tests.

Feb 14 2023, 5:40 AM · Restricted Project, Restricted Project

Feb 1 2023

alexfh accepted D143071: [libc++] Avoid ODR violations in __exception_guard.

The implementation with two different class names seems much cleaner. Thanks!

Feb 1 2023, 10:35 AM · Restricted Project, Restricted Project
alexfh added inline comments to D143071: [libc++] Avoid ODR violations in __exception_guard.
Feb 1 2023, 8:26 AM · Restricted Project, Restricted Project
alexfh added a comment to D133661: [libc++] Improve binary size when using __transaction.

I've uploaded D143071 to fix the problem. I talk to Louis later today, so a fix should be commited within the next 6 hours or so.

Feb 1 2023, 8:19 AM · Restricted Project, Restricted Project
alexfh added a comment to D133661: [libc++] Improve binary size when using __transaction.

@philnik could you commit one of the proposed abi_tag fixes?

Feb 1 2023, 6:02 AM · Restricted Project, Restricted Project
alexfh committed rG561105fb9d3a: [libc++] Fix ODR violation with __exception_guard in mixed exceptions builds (authored by alexfh).
[libc++] Fix ODR violation with __exception_guard in mixed exceptions builds
Feb 1 2023, 6:01 AM · Restricted Project, Restricted Project
alexfh added a comment to D133661: [libc++] Improve binary size when using __transaction.

@philnik could you commit one of the proposed abi_tag fixes?

Feb 1 2023, 4:27 AM · Restricted Project, Restricted Project

Jan 31 2023

alexfh added a comment to D133661: [libc++] Improve binary size when using __transaction.

The ABI tag approach fixes the ODR issue I was looking at

@@ -60,7 +60,7 @@

 #ifndef _LIBCPP_NO_EXCEPTIONS
 template <class _Rollback>
-struct __exception_guard {
+struct __attribute__((__abi_tag__(("fexceptions")))) __exception_guard {
   __exception_guard() = delete;

   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 explicit __exception_guard(_Rollback __rollback)
@@ -89,7 +89,7 @@
 };
 #else  // _LIBCPP_NO_EXCEPTIONS
 template <class _Rollback>
-struct __exception_guard {
+struct __attribute__((__abi_tag__(("fnoexceptions")))) __exception_guard  {
   __exception_guard() = delete;
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_NODEBUG explicit __exception_guard(_Rollback) {}
Jan 31 2023, 5:11 PM · Restricted Project, Restricted Project
alexfh added a comment to D133661: [libc++] Improve binary size when using __transaction.

We're seeing a lot of fallout from this patch and they all look related to the ODR violations that seem to be intentionally added here: both the patch description and the comment for the __exception_guard mention explicitly that combinations of code compiled with exceptions and without exceptions are common.

Am I missing something here?

Do you have a specific example?

Jan 31 2023, 5:53 AM · Restricted Project, Restricted Project

Jan 26 2023

alexfh added a comment to D139235: Reapply "[libc++][ranges]Refactor `copy{,_backward}` and `move{,_backward}`".

We've found the cause of the problems @asbirlea reported above: violation of aliasing rules. Something similar to this snippet: https://gcc.godbolt.org/z/3Y9GEMW4e

Jan 26 2023, 7:33 PM · Restricted Project, Restricted Project
alexfh added a comment to D140158: [CUDA] Allow targeting NVPTX directly without a host toolchain.

This patch breaks our cuda compilations. The output file isn't created after it:

$ echo 'extern "C" __attribute__((global)) void q() {}' >q.cc
$ good-clang \
    -nocudainc -x cuda \
    --cuda-path=somepath/cuda/ \
    -Wno-unknown-cuda-version --cuda-device-only \
    -c q.cc -o qqq-good.o \
  && bad-clang \
    -nocudainc -x cuda \
    --cuda-path=somepath/cuda/ \
    -Wno-unknown-cuda-version --cuda-device-only \
    -c q.cc -o qqq-bad.o \
  && ls qqq*.o
qqq-good.o

https://github.com/llvm/llvm-project/issues/60301 Still broken after this fix?

Jan 26 2023, 7:54 AM · Restricted Project, Restricted Project
alexfh added a comment to D140158: [CUDA] Allow targeting NVPTX directly without a host toolchain.

This patch breaks our cuda compilations. The output file isn't created after it:

$ echo 'extern "C" __attribute__((global)) void q() {}' >q.cc
$ good-clang \
    -nocudainc -x cuda \
    --cuda-path=somepath/cuda/ \
    -Wno-unknown-cuda-version --cuda-device-only \
    -c q.cc -o qqq-good.o \
  && bad-clang \
    -nocudainc -x cuda \
    --cuda-path=somepath/cuda/ \
    -Wno-unknown-cuda-version --cuda-device-only \
    -c q.cc -o qqq-bad.o \
  && ls qqq*.o
qqq-good.o
Jan 26 2023, 7:26 AM · Restricted Project, Restricted Project

Jan 25 2023

alexfh accepted D142500: Fix one of the regressions found in revert of concept sugaring.

Thanks for preparing this fix! I've verified that it fixes the original reproducer as well.

Jan 25 2023, 2:32 AM · Restricted Project, Restricted Project

Jan 24 2023

alexfh added a comment to rG12cb1cb3720d: Revert "[clang] Instantiate concepts with sugared template arguments".

DO NOT revert this, doing so causes about 1/2 our CI to stop working.

Jan 24 2023, 10:54 AM · Restricted Project, Restricted Project
alexfh added a comment to rG12cb1cb3720d: Revert "[clang] Instantiate concepts with sugared template arguments".

It looks like there's no easy fix for now, right? I'm about to revert this commit (12cb1cb3720de8d164196010123ce1a8901d8122) to unblock us.

Jan 24 2023, 10:31 AM · Restricted Project, Restricted Project
alexfh added a comment to rG12cb1cb3720d: Revert "[clang] Instantiate concepts with sugared template arguments".

There's another interesting issue (reduced from https://unicode-org.github.io/icu-docs/apidoc/dev/icu4c/stringpiece_8h_source.html) I bisected to this commit: https://gcc.godbolt.org/z/W5x1s9KbK

Jan 24 2023, 9:46 AM · Restricted Project, Restricted Project

Jan 23 2023

alexfh added a comment to D140939: [X86] Transform AtomicRMW logic operations to BT{R|C|S} if only changing/testing a single bit..

Unfortunately, there's another problem, which doesn't get fixed by https://reviews.llvm.org/D142166 / 2e25204779e5b972d668bf66a0014c1325813b35:

assert.h assertion failed at llvm/lib/Target/X86/X86ISelLowering.cpp:31466 in std::pair<Value *, BitTestKind> FindSingleBitChange(Value *): I != nullptr
    @     0x56257abc9524  __assert_fail
    @     0x562578c7fd1c  FindSingleBitChange()
    @     0x562578c7f6dd  llvm::X86TargetLowering::shouldExpandLogicAtomicRMWInIR()
    @     0x562578c8098b  llvm::X86TargetLowering::shouldExpandAtomicRMWInIR()
    @     0x5625790a6099  (anonymous namespace)::AtomicExpand::tryExpandAtomicRMW()
    @     0x5625790a59dd  (anonymous namespace)::AtomicExpand::runOnFunction()
    @     0x56257a86e87d  llvm::FPPassManager::runOnFunction()
    @     0x56257a876104  llvm::FPPassManager::runOnModule()
    @     0x56257a86ef7c  llvm::legacy::PassManagerImpl::run()
    @     0x562575bad581  clang::EmitBackendOutput()
    @     0x562575baabe9  clang::BackendConsumer::HandleTranslationUnit()
    @     0x562576a85e5c  clang::ParseAST()
    @     0x5625767cbf23  clang::FrontendAction::Execute()
    @     0x562576741cad  clang::CompilerInstance::ExecuteAction()
    @     0x5625757871e8  clang::ExecuteCompilerInvocation()
    @     0x56257577ae41  cc1_main()
    @     0x562575776ec8  ExecuteCC1Tool()
    @     0x5625768ed317  llvm::function_ref<>::callback_fn<>()
    @     0x56257aa23d20  llvm::CrashRecoveryContext::RunSafely()
    @     0x5625768ecb63  clang::driver::CC1Command::Execute()
    @     0x5625768ab2e6  clang::driver::Compilation::ExecuteCommand()
    @     0x5625768ab60f  clang::driver::Compilation::ExecuteJobs()
    @     0x5625768caf70  clang::driver::Driver::ExecuteCompilation()
    @     0x562575776027  clang_main()
    @     0x7f8f04be3633  __libc_start_main
    @     0x562575772bea  _start

I'd suggest to revert while investigating. I'm working on an isolated test case.

$ cat reduced.ll
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

define weak_odr void @f() {
entry:
  br label %if.end19

cont11:                                           ; No predecessors!
  %not = xor i32 0, -1
  %0 = atomicrmw and ptr null, i32 %not monotonic, align 4
  %and13 = and i32 %0, 0
  br label %if.end19

if.end19:                                         ; preds = %cont11, %entry
  ret void
}
$ ./good-clang -c reduced.ll -o good.o
$ ./bad-clang -c reduced.ll -o bad.o
assert.h assertion failed at llvm/lib/Target/X86/X86ISelLowering.cpp:31466 in std::pair<Value *, BitTestKind> FindSingleBitChange(Value *): I != nullptr
    @     0x559d015c9524  __assert_fail
    @     0x559cff67fd1c  FindSingleBitChange()
    @     0x559cff67f6dd  llvm::X86TargetLowering::shouldExpandLogicAtomicRMWInIR()
    @     0x559cff68098b  llvm::X86TargetLowering::shouldExpandAtomicRMWInIR()
    @     0x559cffaa6099  (anonymous namespace)::AtomicExpand::tryExpandAtomicRMW()
    @     0x559cffaa59dd  (anonymous namespace)::AtomicExpand::runOnFunction()
    @     0x559d0126e87d  llvm::FPPassManager::runOnFunction()
    @     0x559d01276104  llvm::FPPassManager::runOnModule()
    @     0x559d0126ef7c  llvm::legacy::PassManagerImpl::run()
    @     0x559cfc5ad581  clang::EmitBackendOutput()
    @     0x559cfc5a94f8  clang::CodeGenAction::ExecuteAction()
    @     0x559cfd1cbf23  clang::FrontendAction::Execute()
    @     0x559cfd141cad  clang::CompilerInstance::ExecuteAction()
    @     0x559cfc1871e8  clang::ExecuteCompilerInvocation()
    @     0x559cfc17ae41  cc1_main()

Thank you for finding the case. A another one (and probably more realistic one is):

define weak_odr void @f(i32 %0) {
entry:
  br label %if.end19

cont11:                                           ; No predecessors!
  %not = xor i32 %0, -1
  %1 = atomicrmw and ptr null, i32 %not monotonic, align 4
  %and13 = and i32 %1, 0
  br label %if.end19

if.end19:                                         ; preds = %cont11, %entry
  ret void
}

So the fix is:

-      assert(I != nullptr);
+
+      // If constant it will fold and we can evaluate later. If its an argument
+      // or something of that nature, we can't analyze.
+      if (I == nullptr)
+        return {nullptr, UndefBit};

I'm happy to post that as a patch.

Created: https://reviews.llvm.org/D142339

But let me know if revert is still preferable (I say as much there).

If you still want to revert, however, happy to do so as well and then repost the full version.

Whichever you think is more prudent.

Jan 23 2023, 3:52 AM · Restricted Project, Restricted Project
alexfh added a comment to D140939: [X86] Transform AtomicRMW logic operations to BT{R|C|S} if only changing/testing a single bit..

Unfortunately, there's another problem, which doesn't get fixed by https://reviews.llvm.org/D142166 / 2e25204779e5b972d668bf66a0014c1325813b35:

assert.h assertion failed at llvm/lib/Target/X86/X86ISelLowering.cpp:31466 in std::pair<Value *, BitTestKind> FindSingleBitChange(Value *): I != nullptr
    @     0x56257abc9524  __assert_fail
    @     0x562578c7fd1c  FindSingleBitChange()
    @     0x562578c7f6dd  llvm::X86TargetLowering::shouldExpandLogicAtomicRMWInIR()
    @     0x562578c8098b  llvm::X86TargetLowering::shouldExpandAtomicRMWInIR()
    @     0x5625790a6099  (anonymous namespace)::AtomicExpand::tryExpandAtomicRMW()
    @     0x5625790a59dd  (anonymous namespace)::AtomicExpand::runOnFunction()
    @     0x56257a86e87d  llvm::FPPassManager::runOnFunction()
    @     0x56257a876104  llvm::FPPassManager::runOnModule()
    @     0x56257a86ef7c  llvm::legacy::PassManagerImpl::run()
    @     0x562575bad581  clang::EmitBackendOutput()
    @     0x562575baabe9  clang::BackendConsumer::HandleTranslationUnit()
    @     0x562576a85e5c  clang::ParseAST()
    @     0x5625767cbf23  clang::FrontendAction::Execute()
    @     0x562576741cad  clang::CompilerInstance::ExecuteAction()
    @     0x5625757871e8  clang::ExecuteCompilerInvocation()
    @     0x56257577ae41  cc1_main()
    @     0x562575776ec8  ExecuteCC1Tool()
    @     0x5625768ed317  llvm::function_ref<>::callback_fn<>()
    @     0x56257aa23d20  llvm::CrashRecoveryContext::RunSafely()
    @     0x5625768ecb63  clang::driver::CC1Command::Execute()
    @     0x5625768ab2e6  clang::driver::Compilation::ExecuteCommand()
    @     0x5625768ab60f  clang::driver::Compilation::ExecuteJobs()
    @     0x5625768caf70  clang::driver::Driver::ExecuteCompilation()
    @     0x562575776027  clang_main()
    @     0x7f8f04be3633  __libc_start_main
    @     0x562575772bea  _start

I'd suggest to revert while investigating. I'm working on an isolated test case.

Jan 23 2023, 12:17 AM · Restricted Project, Restricted Project

Jan 22 2023

alexfh added a comment to D140939: [X86] Transform AtomicRMW logic operations to BT{R|C|S} if only changing/testing a single bit..

Unfortunately, there's another problem, which doesn't get fixed by https://reviews.llvm.org/D142166 / 2e25204779e5b972d668bf66a0014c1325813b35:

assert.h assertion failed at llvm/lib/Target/X86/X86ISelLowering.cpp:31466 in std::pair<Value *, BitTestKind> FindSingleBitChange(Value *): I != nullptr
    @     0x56257abc9524  __assert_fail
    @     0x562578c7fd1c  FindSingleBitChange()
    @     0x562578c7f6dd  llvm::X86TargetLowering::shouldExpandLogicAtomicRMWInIR()
    @     0x562578c8098b  llvm::X86TargetLowering::shouldExpandAtomicRMWInIR()
    @     0x5625790a6099  (anonymous namespace)::AtomicExpand::tryExpandAtomicRMW()
    @     0x5625790a59dd  (anonymous namespace)::AtomicExpand::runOnFunction()
    @     0x56257a86e87d  llvm::FPPassManager::runOnFunction()
    @     0x56257a876104  llvm::FPPassManager::runOnModule()
    @     0x56257a86ef7c  llvm::legacy::PassManagerImpl::run()
    @     0x562575bad581  clang::EmitBackendOutput()
    @     0x562575baabe9  clang::BackendConsumer::HandleTranslationUnit()
    @     0x562576a85e5c  clang::ParseAST()
    @     0x5625767cbf23  clang::FrontendAction::Execute()
    @     0x562576741cad  clang::CompilerInstance::ExecuteAction()
    @     0x5625757871e8  clang::ExecuteCompilerInvocation()
    @     0x56257577ae41  cc1_main()
    @     0x562575776ec8  ExecuteCC1Tool()
    @     0x5625768ed317  llvm::function_ref<>::callback_fn<>()
    @     0x56257aa23d20  llvm::CrashRecoveryContext::RunSafely()
    @     0x5625768ecb63  clang::driver::CC1Command::Execute()
    @     0x5625768ab2e6  clang::driver::Compilation::ExecuteCommand()
    @     0x5625768ab60f  clang::driver::Compilation::ExecuteJobs()
    @     0x5625768caf70  clang::driver::Driver::ExecuteCompilation()
    @     0x562575776027  clang_main()
    @     0x7f8f04be3633  __libc_start_main
    @     0x562575772bea  _start
Jan 22 2023, 11:48 PM · Restricted Project, Restricted Project
alexfh added a comment to D140939: [X86] Transform AtomicRMW logic operations to BT{R|C|S} if only changing/testing a single bit..

Unfortunately, there's another problem, which doesn't get fixed by https://reviews.llvm.org/D142166 / 2e25204779e5b972d668bf66a0014c1325813b35:

assert.h assertion failed at llvm/lib/Target/X86/X86ISelLowering.cpp:31466 in std::pair<Value *, BitTestKind> FindSingleBitChange(Value *): I != nullptr
    @     0x56257abc9524  __assert_fail
    @     0x562578c7fd1c  FindSingleBitChange()
    @     0x562578c7f6dd  llvm::X86TargetLowering::shouldExpandLogicAtomicRMWInIR()
    @     0x562578c8098b  llvm::X86TargetLowering::shouldExpandAtomicRMWInIR()
    @     0x5625790a6099  (anonymous namespace)::AtomicExpand::tryExpandAtomicRMW()
    @     0x5625790a59dd  (anonymous namespace)::AtomicExpand::runOnFunction()
    @     0x56257a86e87d  llvm::FPPassManager::runOnFunction()
    @     0x56257a876104  llvm::FPPassManager::runOnModule()
    @     0x56257a86ef7c  llvm::legacy::PassManagerImpl::run()
    @     0x562575bad581  clang::EmitBackendOutput()
    @     0x562575baabe9  clang::BackendConsumer::HandleTranslationUnit()
    @     0x562576a85e5c  clang::ParseAST()
    @     0x5625767cbf23  clang::FrontendAction::Execute()
    @     0x562576741cad  clang::CompilerInstance::ExecuteAction()
    @     0x5625757871e8  clang::ExecuteCompilerInvocation()
    @     0x56257577ae41  cc1_main()
    @     0x562575776ec8  ExecuteCC1Tool()
    @     0x5625768ed317  llvm::function_ref<>::callback_fn<>()
    @     0x56257aa23d20  llvm::CrashRecoveryContext::RunSafely()
    @     0x5625768ecb63  clang::driver::CC1Command::Execute()
    @     0x5625768ab2e6  clang::driver::Compilation::ExecuteCommand()
    @     0x5625768ab60f  clang::driver::Compilation::ExecuteJobs()
    @     0x5625768caf70  clang::driver::Driver::ExecuteCompilation()
    @     0x562575776027  clang_main()
    @     0x7f8f04be3633  __libc_start_main
    @     0x562575772bea  _start
Jan 22 2023, 11:46 PM · Restricted Project, Restricted Project

Jan 12 2023

alexfh added a comment to rG4def99e64280: [InstCombine] Try to fold `not` into `cmp` iff other users of `cmp` are freely….

We're still actively working on this and so far we had to revert all the five commits locally while we're trying to find a solution. It would still help us, if it was a single commit, not five of them (i.e. I'd appreciate, if you could revert them either together or one by one, and then reland them as a single commit), but that's just for our convenience. I'll also understand, if you reject, since, well, there's nothing wrong with the code.

Jan 12 2023, 6:25 PM · Restricted Project, Restricted Project
alexfh added a comment to D139275: [SimplifyCFG] `FoldBranchToCommonDest()`: deal with mismatched IV's in PHI's in common successor block.

FYI, this change caused widespread performance regressions in our testing. We haven't investigated further since the change was reverted.

I can re-run these tests and do some analysis on the next revision once you decide to reapply.

At this point this is waiting on @alexfh's reproducer mostly.

Jan 12 2023, 12:59 AM · Restricted Project, Restricted Project

Jan 11 2023

alexfh added a comment to rG9ddff66d0c9c: [InstCombine] Fold nested selects.

My little investigation shows that most of our problems were fixed at 07ecdd9b1a8af51f07d5f4dfe46845c801482a39, but some were fixed even before that.

Jan 11 2023, 7:29 PM · Restricted Project, Restricted Project
alexfh added a comment to rG9ddff66d0c9c: [InstCombine] Fold nested selects.

My little investigation shows that most of our problems were fixed at 07ecdd9b1a8af51f07d5f4dfe46845c801482a39, but some were fixed even before that.

Jan 11 2023, 7:21 PM · Restricted Project, Restricted Project
alexfh added a comment to rG9ddff66d0c9c: [InstCombine] Fold nested selects.

@alexfh

It took significantly longer than I expected, but this is what I came up with. The code below changes behavior when compiled with this commit:

$ cat reduced.ll
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

%"class.std::__u::basic_ostream" = type { ptr, %"class.std::__u::basic_ios.base" }
%"class.std::__u::basic_ios.base" = type <{ %"class.std::__u::ios_base", ptr, i32 }>
%"class.std::__u::ios_base" = type { ptr, i32, i64, i64, i32, i32, ptr, ptr, ptr, ptr, i64, i64, ptr, i64, i64, ptr, i64, i64 }

@_ZNSt3__u4coutE = external global %"class.std::__u::basic_ostream"

define i32 @main(i1 %0, i1 %1) {
  %3 = xor i1 %0, %1
  %4 = or i1 %1, %0
  %5 = select i1 %3, i64 5950, i64 0
  %6 = select i1 %4, i64 %5, i64 1
  %7 = trunc i64 %6 to i32
  %8 = tail call ptr @_ZNSt3__u13basic_ostreamIcNS_11char_traitsIcEEElsEi(ptr @_ZNSt3__u4coutE, i32 %7)
  ret i32 0
}

declare ptr @_ZNSt3__u13basic_ostreamIcNS_11char_traitsIcEEElsEi(ptr, i32)

$ good-clang reduced.ll -O3 -static-libgcc ./libc++.a -o q1 && ./q1
5950
$ bad-clang reduced.ll -O3 -static-libgcc ./libc++.a -o q1 && ./q1
0

Please let me know if this is enough.

*THANK YOU*
But, it appears i have not-great news: if i just revert d73383c145ea83d25063246e0c34f5a41fd35293, this miscompile does not happen.
BUT if i additionally revert the 07ecdd9b1a8af51f07d5f4dfe46845c801482a39, which was a fix for

@joanahalili has posted the reproducer for the problems with the original commit on Dec 16th: https://reviews.llvm.org/D139275#4001580.

then it's back.

So in other words, i believe the miscompile you were seeing was already fixed.
Would you like to double-check that the issue you were seeing does not reappear on main with revert of d73383c145ea83d25063246e0c34f5a41fd35293?

Just reverting d73383c145ea83d25063246e0c34f5a41fd35293 (on top of ac1ffd3caca12c254e0b8c847aa8ce8e51b6cfbf) doesn't show any of the problems we found after 9ddff66d0c9c3e18d56e6b20aa26a2a8cdfb6d2b#1160877. So, I guess, it maybe due to the fact that 428f36401b1b695fd501ebfdc8773bed8ced8d4e was reverted and is not yet in the ToT?

Jan 11 2023, 6:36 PM · Restricted Project, Restricted Project
alexfh added a comment to rG9ddff66d0c9c: [InstCombine] Fold nested selects.

@alexfh

It took significantly longer than I expected, but this is what I came up with. The code below changes behavior when compiled with this commit:

$ cat reduced.ll
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

%"class.std::__u::basic_ostream" = type { ptr, %"class.std::__u::basic_ios.base" }
%"class.std::__u::basic_ios.base" = type <{ %"class.std::__u::ios_base", ptr, i32 }>
%"class.std::__u::ios_base" = type { ptr, i32, i64, i64, i32, i32, ptr, ptr, ptr, ptr, i64, i64, ptr, i64, i64, ptr, i64, i64 }

@_ZNSt3__u4coutE = external global %"class.std::__u::basic_ostream"

define i32 @main(i1 %0, i1 %1) {
  %3 = xor i1 %0, %1
  %4 = or i1 %1, %0
  %5 = select i1 %3, i64 5950, i64 0
  %6 = select i1 %4, i64 %5, i64 1
  %7 = trunc i64 %6 to i32
  %8 = tail call ptr @_ZNSt3__u13basic_ostreamIcNS_11char_traitsIcEEElsEi(ptr @_ZNSt3__u4coutE, i32 %7)
  ret i32 0
}

declare ptr @_ZNSt3__u13basic_ostreamIcNS_11char_traitsIcEEElsEi(ptr, i32)

$ good-clang reduced.ll -O3 -static-libgcc ./libc++.a -o q1 && ./q1
5950
$ bad-clang reduced.ll -O3 -static-libgcc ./libc++.a -o q1 && ./q1
0

Please let me know if this is enough.

*THANK YOU*
But, it appears i have not-great news: if i just revert d73383c145ea83d25063246e0c34f5a41fd35293, this miscompile does not happen.
BUT if i additionally revert the 07ecdd9b1a8af51f07d5f4dfe46845c801482a39, which was a fix for

@joanahalili has posted the reproducer for the problems with the original commit on Dec 16th: https://reviews.llvm.org/D139275#4001580.

then it's back.

So in other words, i believe the miscompile you were seeing was already fixed.
Would you like to double-check that the issue you were seeing does not reappear on main with revert of d73383c145ea83d25063246e0c34f5a41fd35293?

Jan 11 2023, 6:23 PM · Restricted Project, Restricted Project

Jan 10 2023

alexfh added a comment to rG4def99e64280: [InstCombine] Try to fold `not` into `cmp` iff other users of `cmp` are freely….

Okay, figured out: reverting just this commit doesn't fix our issue, but reverting this commit and in addition four later related commits makes our code work again:

But again, these five commits can't be reverted cleanly, since there were multiple other changes to the code and tests. It would be nice, if you could do this. We're actively working on an isolated test case and trying to understand what exactly is wrong. So far we haven't found a clear change in the semantic of the generated PTX code, but there's definitely something fishy (maybe further down the road - in ptxas or even in the hardware).

Sorry for lack of feedback here.
Before i do that, could you please try if this affects the outcome:

diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 48f00fac2d0d..fd3943283e99 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -948,7 +948,8 @@ Value *InstCombinerImpl::SimplifySelectsFeedingBinaryOp(BinaryOperator &I,
 /// Freely adapt every user of V as-if V was changed to !V.
 /// WARNING: only if canFreelyInvertAllUsersOf() said this can be done.
 void InstCombinerImpl::freelyInvertAllUsersOf(Value *I, Value *IgnoredUser) {
-  for (User *U : make_early_inc_range(I->users())) {
+  SmallVector<User*> Users(I->user_begin(), I->user_end());
+  for (User *U : Users) {
     if (U == IgnoredUser)
       continue; // Don't consider this user.
     switch (cast<Instruction>(U)->getOpcode()) {
@@ -962,6 +963,7 @@ void InstCombinerImpl::freelyInvertAllUsersOf(Value *I, Value *IgnoredUser) {
       cast<BranchInst>(U)->swapSuccessors(); // swaps prof metadata too
       break;
     case Instruction::Xor:
+      assert(match(U, m_Not(m_Value())));
       replaceInstUsesWith(cast<Instruction>(*U), I);
       break;
     default:
Jan 10 2023, 8:52 AM · Restricted Project, Restricted Project

Jan 6 2023

alexfh added a comment to rG4def99e64280: [InstCombine] Try to fold `not` into `cmp` iff other users of `cmp` are freely….
r8 = (r5 != 0) ? 3 : 0;
if ((r5 != 0) || p1) goto L__BB0_5;

to

r8 = (r5 != 0) ? 3 : 0;
if ((r5 | r1) != 0) goto L__BB0_5;

Actually, that may be over-reduced and likely doesn't demonstrate a problem =\

If you are *sure* this happens on current main, please feel free to revert in the meanwhile.
That particular branch merge thing happens in simplifycfg, so i kinda expect that the change only exposes issue elsewhere.

I'm sure something wrong happens after this patch (and isn't fixed up to a quite recent LLVM commit). I'm also quite confident this affects the nvidia target (just verified that the host compilation isn't affected for the test in question). However, I can't explain what exactly the problem is. And I'm afraid it's not easy for me to revert this commit at this point - it needs to be done quite carefully due to the multiple changes to llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp that were made afterwards. If you can do this, that would help us a lot.

Please hold on. No need to revert this yet. I might have found some issues with my analysis. I'll be back with more details a bit later.

Jan 6 2023, 4:10 PM · Restricted Project, Restricted Project
alexfh added a comment to rG4def99e64280: [InstCombine] Try to fold `not` into `cmp` iff other users of `cmp` are freely….
r8 = (r5 != 0) ? 3 : 0;
if ((r5 != 0) || p1) goto L__BB0_5;

to

r8 = (r5 != 0) ? 3 : 0;
if ((r5 | r1) != 0) goto L__BB0_5;

Actually, that may be over-reduced and likely doesn't demonstrate a problem =\

If you are *sure* this happens on current main, please feel free to revert in the meanwhile.
That particular branch merge thing happens in simplifycfg, so i kinda expect that the change only exposes issue elsewhere.

I'm sure something wrong happens after this patch (and isn't fixed up to a quite recent LLVM commit). I'm also quite confident this affects the nvidia target (just verified that the host compilation isn't affected for the test in question). However, I can't explain what exactly the problem is. And I'm afraid it's not easy for me to revert this commit at this point - it needs to be done quite carefully due to the multiple changes to llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp that were made afterwards. If you can do this, that would help us a lot.

Jan 6 2023, 12:13 PM · Restricted Project, Restricted Project

Jan 5 2023

alexfh added a comment to rG4def99e64280: [InstCombine] Try to fold `not` into `cmp` iff other users of `cmp` are freely….
r8 = (r5 != 0) ? 3 : 0;
if ((r5 != 0) || p1) goto L__BB0_5;

to

r8 = (r5 != 0) ? 3 : 0;
if ((r5 | r1) != 0) goto L__BB0_5;

Actually, that may be over-reduced and likely doesn't demonstrate a problem =\

If you are *sure* this happens on current main, please feel free to revert in the meanwhile.
That particular branch merge thing happens in simplifycfg, so i kinda expect that the change only exposes issue elsewhere.

Jan 5 2023, 7:20 PM · Restricted Project, Restricted Project
alexfh added a comment to rG4def99e64280: [InstCombine] Try to fold `not` into `cmp` iff other users of `cmp` are freely….
r8 = (r5 != 0) ? 3 : 0;
if ((r5 != 0) || p1) goto L__BB0_5;

to

r8 = (r5 != 0) ? 3 : 0;
if ((r5 | r1) != 0) goto L__BB0_5;
Jan 5 2023, 12:38 PM · Restricted Project, Restricted Project
alexfh updated subscribers of rG4def99e64280: [InstCombine] Try to fold `not` into `cmp` iff other users of `cmp` are freely….

Let me take a look...
Is that with the "D139275: [SimplifyCFG] FoldBranchToCommonDest(): deal with mismatched IV's in PHI's in common successor block" reverted?

Jan 5 2023, 12:31 PM · Restricted Project, Restricted Project
alexfh added a comment to rG4def99e64280: [InstCombine] Try to fold `not` into `cmp` iff other users of `cmp` are freely….

Heads up: I suspect that this commit breaks the nvptx64-nvidia-cuda target. I have a few cuda files that stop working correctly after this commit, and I'm reducing one of the tests.

Will take a look the moment there is a repro.
You are sure this is still happening in main, right?
There was recently a fix in this area - 30af2e31917021f68a3d16ee71335f048c9e6235

Jan 5 2023, 10:58 AM · Restricted Project, Restricted Project
alexfh added a comment to rG4def99e64280: [InstCombine] Try to fold `not` into `cmp` iff other users of `cmp` are freely….

Heads up: I suspect that this commit breaks the nvptx64-nvidia-cuda target. I have a few cuda files that stop working correctly after this commit, and I'm reducing one of the tests.

Jan 5 2023, 8:49 AM · Restricted Project, Restricted Project

Jan 4 2023

alexfh added a comment to D140100: [SLP]Integrate looking through shuffles logic into ShuffleInstructionBuilder..
Jan 4 2023, 1:50 PM · Restricted Project, Restricted Project
alexfh added a comment to D140100: [SLP]Integrate looking through shuffles logic into ShuffleInstructionBuilder..

Heads-up: This has caused a change in behavior when using #pragma unroll in some test using SIMD. In other words, using #pragma unroll results in a different output than without it. I'm preparing a reduced test case.

I've got a reduced IR file for that case:

$ old-clang -O2 q.ll && ./a.out
0.0
$ new-clang -O2 q.ll && ./a.out
5.0

Please fix or revert the patch, if there's no trivial fix.

Thanks!

Thanks for the reproducer, will investigate it.

Alexey, could you revert the commit in the meantime?

It is not so easy, there are several commits on top of it already. I just started looking at it, hope to provide a fix in 1-2hrs, if it will take more time, I'll revert it.

Jan 4 2023, 7:58 AM · Restricted Project, Restricted Project
alexfh added a comment to D140100: [SLP]Integrate looking through shuffles logic into ShuffleInstructionBuilder..

Heads-up: This has caused a change in behavior when using #pragma unroll in some test using SIMD. In other words, using #pragma unroll results in a different output than without it. I'm preparing a reduced test case.

I've got a reduced IR file for that case:

$ old-clang -O2 q.ll && ./a.out
0.0
$ new-clang -O2 q.ll && ./a.out
5.0

Please fix or revert the patch, if there's no trivial fix.

Thanks!

Thanks for the reproducer, will investigate it.

Jan 4 2023, 7:52 AM · Restricted Project, Restricted Project

Jan 3 2023

alexfh added a comment to D140100: [SLP]Integrate looking through shuffles logic into ShuffleInstructionBuilder..

Heads-up: This has caused a change in behavior when using #pragma unroll in some test using SIMD. In other words, using #pragma unroll results in a different output than without it. I'm preparing a reduced test case.

Jan 3 2023, 1:35 PM · Restricted Project, Restricted Project

Dec 23 2022

alexfh added a comment to rG9ddff66d0c9c: [InstCombine] Fold nested selects.

It took significantly longer than I expected, but this is what I came up with. The code below changes behavior when compiled with this commit:

Dec 23 2022, 9:07 PM · Restricted Project, Restricted Project

Dec 21 2022

alexfh added a comment to rG9ddff66d0c9c: [InstCombine] Fold nested selects.

I've gone ahead and reverted both potential problem candidates,
WITH implicit assumption that an actionable reproducer will be provided.
Thanks.

Dec 21 2022, 11:07 PM · Restricted Project, Restricted Project

Dec 19 2022

alexfh added a comment to rG9ddff66d0c9c: [InstCombine] Fold nested selects.

Hi Roman, this might be causing some miscompiles in our code. I'm now trying to reduce the test case. I'll notify you when I have more information.

Dec 19 2022, 6:53 AM · Restricted Project, Restricted Project

Dec 16 2022

alexfh added a comment to D139275: [SimplifyCFG] `FoldBranchToCommonDest()`: deal with mismatched IV's in PHI's in common successor block.

I will revert this patch. Please ensure the provided test case works fine before recommitting.

Thanks for the reproducer. Will revert in a sec.

Dec 16 2022, 8:28 AM · Restricted Project, Restricted Project
alexfh added a reverting change for rG1bd0b82e508d: [SimplifyCFG] `FoldBranchToCommonDest()`: deal with mismatched IV's in PHI's in…: rG37b8f09a4b61: Revert "[SimplifyCFG] `FoldBranchToCommonDest()`: deal with mismatched IV's in….
Dec 16 2022, 8:24 AM · Restricted Project, Restricted Project
alexfh committed rG37b8f09a4b61: Revert "[SimplifyCFG] `FoldBranchToCommonDest()`: deal with mismatched IV's in… (authored by alexfh).
Revert "[SimplifyCFG] `FoldBranchToCommonDest()`: deal with mismatched IV's in…
Dec 16 2022, 8:24 AM · Restricted Project, Restricted Project
alexfh added a reverting change for D139275: [SimplifyCFG] `FoldBranchToCommonDest()`: deal with mismatched IV's in PHI's in common successor block: rG37b8f09a4b61: Revert "[SimplifyCFG] `FoldBranchToCommonDest()`: deal with mismatched IV's in….
Dec 16 2022, 8:23 AM · Restricted Project, Restricted Project
alexfh added a comment to D139275: [SimplifyCFG] `FoldBranchToCommonDest()`: deal with mismatched IV's in PHI's in common successor block.

I will revert this patch. Please ensure the provided test case works fine before recommitting.

Dec 16 2022, 8:17 AM · Restricted Project, Restricted Project

Dec 6 2022

alexfh added a comment to D138774: Enhance stack protector.

There's another problem with this patch. Please ensure it's fixed before recommitting. Clang hangs while compiling the following short snippet reduced from an open-source library (compiled on x86-64, linux):

Dec 6 2022, 5:36 AM · Restricted Project, Restricted Project
alexfh added a comment to D139254: Enhance stack protector.

There's a problem in the previous iteration of this patch (https://reviews.llvm.org/D138774). It made clang hang while compiling the following short snippet reduced from an open-source library (compiled on x86-64, linux):

$ cat q.c
char *texsubexpr(char *expression, char *subexpr) {
  char *texchar();
  if (*expression) return texchar(expression, subexpr);
}
Dec 6 2022, 4:27 AM · Restricted Project, Restricted Project

Dec 3 2022

alexfh requested review of D139262: Fix name style. NFC..
Dec 3 2022, 3:55 PM · Restricted Project, Restricted Project
alexfh committed rGc95922c71797: Remove a useless temporary of a base class type. (authored by alexfh).
Remove a useless temporary of a base class type.
Dec 3 2022, 3:01 PM · Restricted Project
alexfh closed D114111: Remove a useless temporary of a base class type..
Dec 3 2022, 3:01 PM · Restricted Project, Restricted Project
alexfh updated the diff for D114111: Remove a useless temporary of a base class type..

Rebased.

Dec 3 2022, 3:00 PM · Restricted Project, Restricted Project
alexfh abandoned D5577: [clang] Optimized ASTNodeKind::isBaseOf.
Dec 3 2022, 2:44 PM · Restricted Project
alexfh abandoned D60868: [clang-format] Fix an assertion failure.
Dec 3 2022, 2:44 PM · Restricted Project, Restricted Project, Restricted Project
alexfh abandoned D32170: Add a FixItHint for -Wmissing-prototypes to insert 'static '..
Dec 3 2022, 2:39 PM · Restricted Project
alexfh abandoned D97288: Added `Follow` parameter to llvm::vfs::FileSystem::status().
Dec 3 2022, 2:34 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
alexfh accepted D138942: Support: Make Wait's SecondsToWait be Optional [NFC].

LGTM modulo open comments.

Dec 3 2022, 2:28 PM · Restricted Project, Restricted Project

Dec 1 2022

alexfh added inline comments to D138942: Support: Make Wait's SecondsToWait be Optional [NFC].
Dec 1 2022, 4:46 PM · Restricted Project, Restricted Project

Nov 29 2022

alexfh added inline comments to D137213: [clang][modules] NFCI: Pragma diagnostic mappings: write/read FileID instead of SourceLocation.
Nov 29 2022, 5:53 PM · Restricted Project, Restricted Project
alexfh added inline comments to D137213: [clang][modules] NFCI: Pragma diagnostic mappings: write/read FileID instead of SourceLocation.
Nov 29 2022, 4:30 PM · Restricted Project, Restricted Project
alexfh added inline comments to D137213: [clang][modules] NFCI: Pragma diagnostic mappings: write/read FileID instead of SourceLocation.
Nov 29 2022, 7:40 AM · Restricted Project, Restricted Project
alexfh updated subscribers of D137213: [clang][modules] NFCI: Pragma diagnostic mappings: write/read FileID instead of SourceLocation.
Nov 29 2022, 7:17 AM · Restricted Project, Restricted Project

Nov 25 2022

alexfh added a reverting change for rGf61c135a6908: [clang][modules] NFCI: Pragma diagnostic mappings: write/read FileID instead of…: rG38f5ab4d44e3: Revert "[clang][modules] NFCI: Pragma diagnostic mappings: write/read FileID….
Nov 25 2022, 8:05 AM · Restricted Project, Restricted Project
alexfh committed rG38f5ab4d44e3: Revert "[clang][modules] NFCI: Pragma diagnostic mappings: write/read FileID… (authored by alexfh).
Revert "[clang][modules] NFCI: Pragma diagnostic mappings: write/read FileID…
Nov 25 2022, 8:05 AM · Restricted Project, Restricted Project
alexfh added a reverting change for D137213: [clang][modules] NFCI: Pragma diagnostic mappings: write/read FileID instead of SourceLocation: rG38f5ab4d44e3: Revert "[clang][modules] NFCI: Pragma diagnostic mappings: write/read FileID….
Nov 25 2022, 8:05 AM · Restricted Project, Restricted Project
alexfh added inline comments to D137213: [clang][modules] NFCI: Pragma diagnostic mappings: write/read FileID instead of SourceLocation.
Nov 25 2022, 6:09 AM · Restricted Project, Restricted Project

Nov 20 2022

alexfh added a comment to D137213: [clang][modules] NFCI: Pragma diagnostic mappings: write/read FileID instead of SourceLocation.

Heads up: this commit has broken compilation in a small number of cases: some #includes from modularized headers now fail to be found. I'm still trying to figure out what exactly happened, but some behavior has definitely changed by this commit.

Nov 20 2022, 12:26 PM · Restricted Project, Restricted Project

Nov 18 2022

alexfh added a comment to D136565: [clang] Instantiate alias templates with sugar.

And another problem with this patch: there's another pattern (or multiple different patterns?) in the code, that result in around 3x clang memory usage increase after this patch. The result of -print-stats doesn't make it clear where the additional allocations come from:

Nov 18 2022, 1:14 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project

Nov 10 2022

alexfh added a comment to D137751: Produce a more informative diagnostics when Clang runs out of source locations.

Thanks for adding this diagnostic! I wonder whether we can include SLoc usage information into -print-stats output?

Nov 10 2022, 3:48 AM · Restricted Project, Restricted Project

Nov 8 2022

alexfh added a reverting change for rG279fe6281d2c: [clang] Instantiate alias templates with sugar: rGa5c18fcf6e7f: Revert "[clang] Instantiate alias templates with sugar".
Nov 8 2022, 8:20 AM · Restricted Project, Restricted Project, Restricted Project
alexfh committed rGa5c18fcf6e7f: Revert "[clang] Instantiate alias templates with sugar" (authored by alexfh).
Revert "[clang] Instantiate alias templates with sugar"
Nov 8 2022, 8:20 AM · Restricted Project, Restricted Project, Restricted Project
alexfh added a reverting change for D136565: [clang] Instantiate alias templates with sugar: rGa5c18fcf6e7f: Revert "[clang] Instantiate alias templates with sugar".
Nov 8 2022, 8:20 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
alexfh added a comment to D136565: [clang] Instantiate alias templates with sugar.

@alexfh Thanks!

While there is a huge increase in the amount of UsingTypes, it seems the total amount is still reasonable and does not explain the perf hit.

Perhaps this is a case of bad hashing and they are all falling into the same bucket?

cc @sam.mcall for awareness of UsingType issue.
It may be some very simple problem, I just can't even look at the code right now.

Nov 8 2022, 4:26 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project

Nov 7 2022

alexfh updated subscribers of D136565: [clang] Instantiate alias templates with sugar.

Given the broad impact of this in our code I'm inclined to revert the patch
to unblock us. The test case I have so far is still too large, but I hope
to get something shareable tomorrow.

Nov 7 2022, 7:13 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
alexfh added a comment to D136565: [clang] Instantiate alias templates with sugar.

Hi Matheus, 279fe6281d2ca5b2318c7437316c28750feaac8d causes compilation timeout on some of our internal files.

Hi @alexfh. It looks like somehow we may be creating a crazy amount of extra UsingTypes in your test scenario.

There is a clang flag that prints some performance statistics, including the amount of AST nodes created. I can't lookup the spelling as I am on vacations / on a cellphone for the next two weeks, but I believe this is documented.

One idea to get a reduction here is to perhaps tie your creduce interestingness test to that UsingType count.

Nov 7 2022, 6:54 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
alexfh added a comment to D136565: [clang] Instantiate alias templates with sugar.

Hi Matheus, 279fe6281d2ca5b2318c7437316c28750feaac8d causes compilation timeout on some of our internal files. We're trying to get a test case we can share, but so far the only information I can provide is compiler perf profile difference:

Nov 7 2022, 5:17 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project

Oct 23 2022

alexfh requested changes to D136539: [Lex] Bring back the magic number 50 in updateConsecutiveMacroArgTokens..

Undo LGTM, since we decided to go with a revert.

Oct 23 2022, 2:17 PM · Restricted Project, Restricted Project
alexfh added a comment to D136539: [Lex] Bring back the magic number 50 in updateConsecutiveMacroArgTokens..

FYI, I've reverted f83347b0bedb22ea676861c8e4e2ed9c31371ade and 74e4f778cf16cbf7163b5c6de6027a43f5e9169f.

Oct 23 2022, 2:15 PM · Restricted Project, Restricted Project
alexfh added a reverting change for rG74e4f778cf16: [Lex] Simplify and cleanup the updateConsecutiveMacroArgTokens implementation.: rGe7656daea872: Revert "[Lex] Simplify and cleanup the updateConsecutiveMacroArgTokens….
Oct 23 2022, 2:15 PM · Restricted Project, Restricted Project
alexfh committed rGe7656daea872: Revert "[Lex] Simplify and cleanup the updateConsecutiveMacroArgTokens… (authored by alexfh).
Revert "[Lex] Simplify and cleanup the updateConsecutiveMacroArgTokens…
Oct 23 2022, 2:15 PM · Restricted Project, Restricted Project
alexfh added a reverting change for rG74e4f778cf16: [Lex] Simplify and cleanup the updateConsecutiveMacroArgTokens implementation.: rGe86161076e4a: Revert "[TokenLexer][NFC] Rename the InstLoc to ExpandLoc".
Oct 23 2022, 2:15 PM · Restricted Project, Restricted Project
alexfh added a reverting change for rGf83347b0bedb: [TokenLexer][NFC] Rename the InstLoc to ExpandLoc: rGe86161076e4a: Revert "[TokenLexer][NFC] Rename the InstLoc to ExpandLoc".
Oct 23 2022, 2:15 PM · Restricted Project, Restricted Project
alexfh committed rGe86161076e4a: Revert "[TokenLexer][NFC] Rename the InstLoc to ExpandLoc" (authored by alexfh).
Revert "[TokenLexer][NFC] Rename the InstLoc to ExpandLoc"
Oct 23 2022, 2:15 PM · Restricted Project, Restricted Project
alexfh added a reverting change for D134942: [Lex] Simplify and cleanup the updateConsecutiveMacroArgTokens implementation.: rGe7656daea872: Revert "[Lex] Simplify and cleanup the updateConsecutiveMacroArgTokens….
Oct 23 2022, 2:15 PM · Restricted Project, Restricted Project
alexfh added a reverting change for D134942: [Lex] Simplify and cleanup the updateConsecutiveMacroArgTokens implementation.: rGe86161076e4a: Revert "[TokenLexer][NFC] Rename the InstLoc to ExpandLoc".
Oct 23 2022, 2:15 PM · Restricted Project, Restricted Project
alexfh added a comment to D136539: [Lex] Bring back the magic number 50 in updateConsecutiveMacroArgTokens..

This is a subtle change that needs careful review, and honestly should have a test.

I realize there's a breakage that needs to be fixed with some urgency and you believe you're just restoring the old behavior, however in that case the right course of action is to revert the patch.

Oct 23 2022, 1:56 PM · Restricted Project, Restricted Project

Oct 22 2022

alexfh accepted D136539: [Lex] Bring back the magic number 50 in updateConsecutiveMacroArgTokens..

LG, thanks!

Oct 22 2022, 3:51 PM · Restricted Project, Restricted Project

Oct 6 2022

alexfh added a comment to D132657: [DSE] Eliminate noop store even through has clobbering between LoadI and StoreI.

We started seeing incorrect behavior in some code after this commit. The test case is here: https://gcc.godbolt.org/z/hhj64noKs.

Oct 6 2022, 8:45 AM · Restricted Project, Restricted Project