Thank you for your clarification. I'll change it.

In D105169#3319773, @dblaikie wrote:

In D105169#3315009, @MaskRay wrote:

It may not be worth changing now, but I want to mention: it's more conventional to have a BoolOption which adds -[no-]noundef-analysis. Since both positive and negative forms exist. When we make the default switch, existing users don't need to change the option. After the option becomes quite stable and the workaround is deemed not useful, we can remove the CC1 option.

+1 to this (changing the name and the default at the same time makes migrations a bit more difficult - if the default is changed without renaming (by having both positive and negative flag names) then users can adopt their current default explicitly with no change ahead of picking up the patch that changes the default) & also this flag seems to have no tests? Could you (@hyeongyukim ) add some frontend test coverage for the flag - and yeah, maybe consider giving it a name that has both explicit on/off names, as @MaskRay suggested? (I think that's useful even after the default switch - since a user might want to override a previous argument on the command line, etc)

@nathanchance Hi, I analyzed all four warnings.

@nathanchance
You were right. I succeeded in reproducing the warning with -fsanitize-coverage=trace-pc
However, this problem seems to be fixed in the latest version. (I used 57a551a)

I tried to reproduce the last warning (intelfbhw_validate_mode), but I failed to produce it.
I think my reproducer is correct, but it does not make any warning.
Can you tell me which part was wrong?

Great. I'll check it out.

In D105169#3211929, @nathanchance wrote:

@hyeongyukim I am currently offline for the evening but it seems like my reduction might have been too aggressive. It looks like this code comes from ravb_set_gti() (https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/renesas/ravb_main.c?h=v5.16-rc7#n2480), which checks that rate is not zero before using it as a divisor. I will see if I can get a reproducer without any undefined behavior such as this.

In D105169#3116810, @nathanchance wrote:

Prior to the latest revert (fd9b099906c61e46574d1ea2d99b973321fe1d21), the Linux kernel's binary verifier (objtool) points out an issue when building with ThinLTO and -fsanitize=integer-divide-by-zero. I have no idea if this is an issue with the tool or this series. A simplified reproducer:

$ cat ravb_main.i
int ravb_set_gti_ndev_rate;
unsigned int ravb_set_gti_ndev_inc;
void ravb_set_gti_ndev() {
  ravb_set_gti_ndev_inc = 1000000000;
  ravb_set_gti_ndev_inc = ravb_set_gti_ndev_inc / ravb_set_gti_ndev_rate;
  if (ravb_set_gti_ndev_inc)
    _dev_err(ravb_set_gti_ndev_inc);
}

$ clang -std=gnu89 -O2 -flto=thin -fsanitize=integer-divide-by-zero -c -o ravb_main.o ravb_main.i

$ llvm-ar cDPrsT ravb.o ravb_main.o

$ ld.lld -m elf_x86_64 -r -o ravb.lto.o --whole-archive ravb.o

$ ./objtool orc generate --no-fp --no-unreachable --retpoline --uaccess --mcount --module ravb.lto.o
ravb.lto.o: warning: objtool: .text.ravb_set_gti_ndev: unexpected end of section

With LLVM 13.0.0, there is no warning with those commands. The original and reduced .i file, interestingness test, and static objtool binary are available here.

In D105169#3115814, @erichkeane wrote:

Either this or D108453 (which were committed together!) caused this assert according to my git-bisect: https://godbolt.org/z/4rqYKfW7K

NOTE that this fails in a lit-test for me, clang CodeGen/ifunc.c (though my downstream seems to run the verifier even with -emit-llvm, so you might need to just swap it to an -emit-obj to get this to repro).

In D105169#3115814, @erichkeane wrote:

Either this or D108453 (which were committed together!) caused this assert according to my git-bisect: https://godbolt.org/z/4rqYKfW7K

NOTE that this fails in a lit-test for me, clang CodeGen/ifunc.c (though my downstream seems to run the verifier even with -emit-llvm, so you might need to just swap it to an -emit-obj to get this to repro).

If you cannot fix this quickly, let me know and I can revert it.

IR for this looks like:

[ekeane1@scsel-clx-24 llvm]$ /localdisk2/ekeane1/workspaces/xmain-web/builds/xmainefi2linux_debug/llvm/bin/clang -cc1 -internal-isystem /localdisk2/ekeane1/workspaces/xmain-web/builds/xmainefi2linux_debug/llvm/lib/clang/14.0.0/include -nostdsysteminc -triple i386-unknown-linux-gnu -emit-llvm -o - /localdisk2/ekeane1/workspaces/xmain-web/llvm/clang/test/CodeGen/ifunc.c -disable-llvm-passes
; ModuleID = '/localdisk2/ekeane1/workspaces/xmain-web/llvm/clang/test/CodeGen/ifunc.c'
source_filename = "/localdisk2/ekeane1/workspaces/xmain-web/llvm/clang/test/CodeGen/ifunc.c"
target datalayout = "e-m:e-p:32:32-p270:32:32-p271:32:32-p272:64:64-f64:32:64-f80:32-n8:16:32-S128"
target triple = "i386-unknown-linux-gnu"

@global = global i32 0, align 4

@foo = ifunc i32 (i32), i32 (i32)* ()* @foo_ifunc
@goo = ifunc void (), bitcast (i8* ()* @goo_ifunc to void ()* ()*)

; Function Attrs: noinline nounwind optnone
define internal i32 (i32)* @foo_ifunc() #0 {
entry:
  %0 = load i32, i32* @global, align 4
  %tobool = icmp ne i32 %0, 0
  %1 = zext i1 %tobool to i64
  %cond = select i1 %tobool, i32 (i32)* @f1, i32 (i32)* @f2
  ret i32 (i32)* %cond
}

; Function Attrs: noinline nounwind optnone
define dso_local i32 @bar() #0 {
entry:
  %call = call i32 @foo(i32 noundef 1)
  ret i32 %call
}

; Function Attrs: noinline nounwind optnone
define dso_local void @bar2() #0 {
entry:
  call void @goo()
  ret void
}

; Function Attrs: noinline nounwind optnone
define dso_local i8* @goo_ifunc() #0 {
entry:
  ret i8* null
}

; Function Attrs: noinline nounwind optnone
define internal i32 @f1(i32 noundef %i) #0 {
entry:
  %i.addr = alloca i32, align 4
  store i32 %i, i32* %i.addr, align 4
  %0 = load i32, i32* %i.addr, align 4
  %add = add nsw i32 %0, 1
  ret i32 %add
}

; Function Attrs: noinline nounwind optnone
define internal i32 @f2(i32 noundef %i) #0 {
entry:
  %i.addr = alloca i32, align 4
  store i32 %i, i32* %i.addr, align 4
  %0 = load i32, i32* %i.addr, align 4
  %add = add nsw i32 %0, 2
  ret i32 %add
}

attributes #0 = { noinline nounwind optnone "frame-pointer"="none" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+cx8,+x87" }

!llvm.module.flags = !{!0, !1}
!llvm.ident = !{!2}

!0 = !{i32 1, !"NumRegisterParameters", i32 0}
!1 = !{i32 1, !"wchar_size", i32 4}
!2 = !{!"Intel(R) oneAPI DPC++/C++ Compiler 2022.1.0 (2022.x.0.YYYYMMDD)"}

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp
index ea3e5bdbc754..826c6d36e1b1 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp
@@ -1360,7 +1360,7 @@ uptr internal_clone(int (*fn)(void *), void *child_stack, int flags, void *arg,
 #elif defined(__aarch64__)
 uptr internal_clone(int (*fn)(void *), void *child_stack, int flags, void *arg,
                     int *parent_tidptr, void *newtls, int *child_tidptr) {
-  long long res;
+  register long long res __asm__("x0");
   if (!fn || !child_stack)
     return -EINVAL;
   CHECK_EQ(0, (uptr)child_stack % 16);

After modifying internal_clone like this, the problem disappeared.
Is it okay to commit this change by myself?

I checked the reason for failure in address sanitizer tests on the 2-stage aarch64 buildbots.
The buildbot failure was occured because the internal_clone function of the compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp file is being compiled incorrectly.
The internal_clone function is a simple function that calls the clone system call of Linux. Its original return value should be the PID of the newly created process, but the actual returned value is 220 (which is the __NR_clone value.)

Rebase

Thank you for the review.
I will also prepare for improvement!

Thank you!
In the meantime, I will rebase this patch and resolve conflicts.

Changed to the original patch.

Right, many codes were not directly related to solving the loop unswitch problem.
Those codes were created to reduce performance regression, and I agree to separate them into different patches.

remove unused sink functions.

Add nontrivial-unswitch-freeze.ll to show changes.

Since the flag that adds the freeze is off, there is no change in the existing test code.
How about adding -freeze-loop-unswitch-cond to some of the tests so that we can see where the freeze is added?

refix test code

update test codes

Add Freeze-loop-unswitch-cond flag.
This flag determines whether to add a freeze to the condition of the loop switch.
When this flag is turned on, the miscompilation is solved, but It will degrade the performance.
The default value of the flag is set to false until the performance problem is completely solved.

Thanks!

reformat code

Add the new CreateInsertElement

In D110311#3026282, @aqjune wrote:

@spatel Thanks!
@hyeongyukim Do you mind if we add the new CreateInsertElements and use them in this patch?

Revert PPCTargetTransformInfo.cpp

Q: Is there any test that is affected by this?

Like D110226, D110227, and D110230, It would be better to change undef to poison in one place.

Correct the wrong rebase.

Rebase

Fix comment, update test file(wasm.c)

reformat code

rebase and change function name.

I just created a patch D110146 that adds a constructor, so I'll close this patch.

Oh, I misunderstood. I'll just add a constructor for one-vector-input.

In D110034#3009544, @spatel wrote:

We have a one-vector-input create function in IRBuilder, so it was easy to update that in one place with D93793. Could we do the same for the raw ShuffleVectorInst constructor?

I did some experiments to confirm the benefits of adding a noundef attribute to the function parameter. (test result link)

This and D105169 are renewed version of the old patch (D82317).
When pushing to the main branch, I'll commit this and D105169 as a single commit to avoid CI failure.