Hi @pengfei, are you happy with the patch now?

Rebase.

Before and after diff of the test:

Hi @tim.schmielau, your assumption that libm-vec expects vectors filled with pointers is correct, as shown by https://elixir.bootlin.com/glibc/glibc-2.37.9000/source/sysdeps/x86_64/fpu/test-vector-abi-sincos.h#L46.

Rebase.

Add test.

Thanks @pengfei and @craig.topper, I'll close this reivew.

I think this is not the right fix. We intended to define legal types under useAVX512Regs.

In D127115#4403083, @deadalnix wrote:

In D127115#4399236, @RKSimon wrote:

define void @_ZN12_GLOBAL__N_114AssemblyWriterC2ERN4llvm21formatted_raw_ostreamERNS1_11SlotTrackerEPKNS1_6ModuleEPNS1_24AssemblyAnnotationWriterEbb() {
entry:
  %__end1.sroa.5.0.copyload = load ptr, ptr poison, align 8
  %__end1.sroa.6.0.copyload = load ptr, ptr null, align 8
  %cmp.i.i.i8.i.i = icmp ne ptr null, %__end1.sroa.6.0.copyload
  %cmp.i.i.i.i9.i.i = icmp ne ptr null, %__end1.sroa.5.0.copyload
  %.not.i = select i1 %cmp.i.i.i8.i.i, i1 true, i1 %cmp.i.i.i.i9.i.i
  %.not.i.fr = freeze i1 %.not.i
  br i1 %.not.i.fr, label %for.cond.us, label %if.end.split

for.cond.us:                                      ; preds = %entry
  unreachable

if.end.split:                                     ; preds = %entry
  ret void
}

I am failing to see how this generates an infinite loop. llc compiles it just fine. Would you have more details on how to repro?

In D127115#4399236, @RKSimon wrote:

define void @_ZN12_GLOBAL__N_114AssemblyWriterC2ERN4llvm21formatted_raw_ostreamERNS1_11SlotTrackerEPKNS1_6ModuleEPNS1_24AssemblyAnnotationWriterEbb() {
entry:
  %__end1.sroa.5.0.copyload = load ptr, ptr poison, align 8
  %__end1.sroa.6.0.copyload = load ptr, ptr null, align 8
  %cmp.i.i.i8.i.i = icmp ne ptr null, %__end1.sroa.6.0.copyload
  %cmp.i.i.i.i9.i.i = icmp ne ptr null, %__end1.sroa.5.0.copyload
  %.not.i = select i1 %cmp.i.i.i8.i.i, i1 true, i1 %cmp.i.i.i.i9.i.i
  %.not.i.fr = freeze i1 %.not.i
  br i1 %.not.i.fr, label %for.cond.us, label %if.end.split

for.cond.us:                                      ; preds = %entry
  unreachable

if.end.split:                                     ; preds = %entry
  ret void
}

Thanks @CarlosAlbertoEnciso.

Address review comments.

Add codeview test.

Address review comments.

Address review comments.

Negative test: https://reviews.llvm.org/D145933

Update diff.

Address review comments

In D133145#3857812, @Chia-hungDuan wrote:

In D133145#3857730, @dyung wrote:

@Chia-hungDuan, your change is causing a build failure on our internal bots that run older versions of gcc, specifically 7.5.0 and 9.4.0. It fails with the following error:

/home/dyung/src/upstream/llvm_clean_git/compiler-rt/lib/scudo/standalone/local_cache.h:190:14: error: conversion from ‘int’ to ‘scudo::u16’ {aka ‘short unsigned int’} may change value [-Werror=conversion]                                        
  190 |     C->Count -= Count;                                                                                                                                                                                                                      
      |     ~~~~~~~~~^~~~~~~~

This looks weird to me. Both of them are u16 already. Can you check why it thinks C-Count is int?

/home/dyung/src/upstream/llvm_clean_git/compiler-rt/lib/scudo/standalone/local_cache.h:154:23: error: conversion from ‘int’ to ‘scudo::u16’ {aka ‘short unsigned int’} may change value [-Werror=conversion]

154 |       P->MaxCount = 2 * TransferBatch::getMaxCached(Size);                                                                                                                                                                                  
    |                     ~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

https://reviews.llvm.org/D135945. I think this will resolve the warning.

Can you take a look and revert if it will take you a while to fix?

In D133145#3858231, @ronlieb wrote:

we are seeing the same issue compiling compiler-rt with these patches in trunk, are the various buildbots catching this also?
for awareness: @saiislam

Fix cost model.

Improve the cost computation and update tests.

Modify AArch64 and RISCV tests.

Add context to diff.

Update failing cost model tests.

Reverted due to test failures, investigating further.

Remove unused included file.

Add context to diff.

Rebase patch and address review comments.

Remove unnecessary attribute from test.

@RKSimon I've created https://reviews.llvm.org/D128178 for the test.

Add context to diff.

Update RUN lines to fix failing builds and add new variable argument test to SLP.

Replace undef with poison.

Address review comments.

Update diff based on reviewer's comments.

Update diff based on reviewer's comments.

Remove FIXME from test.

Add context to diff.

Update diff based on review comments.

Updating the diff for completeness because D126055. Still looking into the reviewers comments.

Address review comments.

Hi @RKSimon, I've updated the tests for the current trunk and split them off into https://reviews.llvm.org/D126055.

Resolve another merge conflict Transforms/LoopIdiom/X86/left-shift-until-bittest.ll.

Transforms/LoopIdiom/X86/left-shift-until-bittest.ll test was recently added in this commit. The test in its current form relies on LCSSA's current behaviour. I have updated the patch to remove that reliance.

In D92576#2451875, @jmorse wrote:

LGTM, although couldn't you also just swap the basictest.ll DEBUGIFY-NEXT to be a DEBUGIFY-NOT: call void @llvm.dbg.value and not add the extra test.

(Nabeel and I share a corporate overloard, we should leave this a little more time in case anyone else wants to chip in).

Added context to diff.