Our out-of-tree backend is currently using EntrySU for working with inter-region latencies.

In D83101#2223698, @lebedev.ri wrote:

In D83101#2223689, @materi wrote:

We have seen some issues with missing symbols during linking that I think are caused by this patch. The origin of this is downstream fuzz testing.

The global variable aglobal is renamed:

$ cat global.ll
@aglobal = dso_local global i16 0, align 1
@b = dso_local local_unnamed_addr global i16 0, align 1

define dso_local void @c() local_unnamed_addr {
entry:
  %d.sroa.0.1.vec.extract = extractelement <4 x i16*> <i16* @aglobal, i16* @aglobal, i16* @aglobal, i16* @aglobal>, i32 1
  %0 = ptrtoint i16* %d.sroa.0.1.vec.extract to i16
  store i16 %0, i16* @b, align 1
  ret void
}

$ opt -scalarizer global.ll -S
; ModuleID = 'global.ll'
source_filename = "global.ll"

@d.sroa.0.1.vec.extract = dso_local global i16 0, align 1
@b = dso_local local_unnamed_addr global i16 0, align 1

define dso_local void @c() local_unnamed_addr {
entry:
  %0 = ptrtoint i16* @d.sroa.0.1.vec.extract to i16
  store i16 %0, i16* @b, align 1
  ret void
}

I see that the global is renamed, https://godbolt.org/z/PM95se, it's not really intentional.
But i think something else is missing in this test - what's the failure? -verify passes

We have seen some issues with missing symbols during linking that I think are caused by this patch. The origin of this is downstream fuzz testing.

In D49671#1285281, @materi wrote:

In D49671#1284997, @MatzeB wrote:

Some words about the different kinds of operands:

The extra operands do make sense semantically and are necessary for our modeling of things. The thing I regret though is that just being an implicit operand can mean two things today: It's an operand that isn't explicitly emitted to assembly/encoded or it's an operand that does not correspond to a read/write access in hardware or both. In this patch we only want to catch the 2nd kind, but not purely cases of the first. While it is unfortunate to not have this modeled as two separate bits today, it feels to me like the heuristic is close enough. Should we try again with an extra MO.isImplicit() in the condition?

I agree that this overloading is unfortunate! How hard would it be to split these operands in two different types?

In D49671#1284997, @MatzeB wrote:

In D49671#1283704, @materi wrote:

I don't like any implementation that has first-class and second-class MachineOperands. At least I think it's a bad idea to have this in the default implementation, doing it in a target hook makes sense though.

I don't think having a target specific hook is good enough here because some of the problematic operands are generated by generic register allocation code; you will get them as soon as you have subregisters in the mix.

In D49671#1283621, @jonpa wrote:

I'm wondering if we could maybe keep the old flexible way to look at MachineOperands and put the functionality which sets the latency to zero in the getOperandLatency hook instead?

@materi: I think your ideas make sense. If you have a patch, could you post it, please?

This patch is causing some problems in my out-of-tree back-end. We add some MachineOperands on the fly for some uses/defs that are conditional or depend on some circumstances, like how registers were allocated, or which depth a loop is at in a loop nest. With this patch, these manually added operands don't work as we intend.

In D38101#901733, @lebedev.ri wrote:

In D38101#901709, @materi wrote:

Hi!

Hi.

After this patch I started to see warnings:

Thank you for the report!

e.c:8:23: warning: integer constant not in range of enumerated type 'enum E' [-Wassign-enum]
  enum E {a = 7,} e = 1000;
                      ^
e.c:10:12: warning: comparison 'enum E' > 7 is always false [-Wtautological-constant-compare]
  return e > 7;

Isn't the the "always false" message misleading? It's only "always false" if e was initialized with an in-range value. Maybe the tautology check should be on the enum's underlying type instead?

I agree, please open a bug, i'll look into this.

After this patch I started to see warnings:

In D29925#687131, @dberlin wrote:

Sorry about that.
I wasn't aware intrinsics were allowed to have a calling convention, since they are internal to llvm.
No intrinsic defined in Intrinsics* has one, from what i can tell, and as you can see, the code we use to upgrade generically does not do anything with calling convention either.
I'm not even sure what it would mean for an intrinsic to have a calling convention (instead of something it *lowers to* to have a calling convention).

I think this commit is causing a bug in my out-of-tree target. From opt -lint I started seeing messages like this:

Reported it here: https://llvm.org/bugs/show_bug.cgi?id=27758

I am debugging an issue with bitfields and debug info and found that this commit is where the problems begin.

This looks like a nice SMS implementation!