The data-flow analysis framework has a concept very similar to IRUnit its called ProgramPoint maybe this should be shared ?

pushed as 2c7b7eca85c2ccc9b0d5c65169213ce499652f92

Thank you for the review.

In D134725#3819217, @leonardchan wrote:

Prior to this patch APFixedPoint only support semantics where the LsbWeight is is negative and the Width is at least as large as -LsbWeight.
This patch remove both those requirements.
for example:
with LsbWeight = 2, 12 would be represented as 3.
with LsbWeight = -2, 12 would be represented as 48.

IIUC should the weights have opposite values in this example? Based off the code, it looks like scale is just negative. So with LsbWeight = -2, then the scale is 2, and 0x1100 (12) is actually 0x11.00 (3). Similarly, LsbWeight = 2, then the scale is -2, and 0x1100 (12) is actually 0x1100|00 (48).

pushed as fa40fd40e055630c34890de84eb048796133fe03

pushed as 688c51a5acc53b456014e53663051476d825e896

has been committed as bb63d249f8612f87e819071663d81f516a2bec74

I would still like a comment for what to do about this issue (which i bypassed):

for the case of SmallVector it is as easy as adding a few used
But for many containers adding used attribute may cause previously new failure because the function annotated cannot be instantiated.

I updated to to have the member version of cast/isa/dyn_cast depend on the out of line ones.

In D132009#3728861, @bzcheeseman wrote:

+1 to the concept, many thanks for this! I think it makes more sense to implement the member versions with the llvm-style versions, it should simplify the member style code because then you won't have to manually implement the classof/isa/cast innards it'd just be cast<T>(*this) or something.

In D132010#3730193, @mehdi_amini wrote:

assert(v2.getType().getWidth() == 8); /// no need to do .cast<IntegerType>()

/// Assuming MyOp is defined to operate on TensorType exclusively
MyOp.getLHS().getType() // is a TensorType

Ah I like this! The API is indeed quite nice when getType does not need to be casted... So I'm supportive here if River is as well :)

:)

(You have a typo in multiple place you wrote IntergerType)

In D132010#3729321, @rriddle wrote:

How many places in-tree would benefit from this? It's hard to gauge what the value of this is from the patch as-is.

In D132010#3728346, @mehdi_amini wrote:

Can you elaborate more on the motivation in the description?
Right now you're touching on it with:

I also think TypedValue can be a useful as to express directly what is expected and remove the need for to cast its Values.

But I'm not sure what you mean, if you could also add examples of what kind of API would take advantage of this?

I didn't realize this existed. thank you for the information.

is there interest in having this upstream ?
I am not sure how it should be tested upstream ?

This seems like a good change to me. but i don't think my approval is enough

In D74130#3085313, @aaron.ballman wrote:

FWIW, I am not seeing double errors on that code. Here's the output I get with this patch applied locally:

F:\source\llvm-project>cat "C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp"
consteval int f1() { return 0; }
consteval auto g() { return f1; }

constexpr auto e = g();
constexpr auto e1 = f1;

F:\source\llvm-project>llvm\out\build\x64-Debug\bin\clang.exe -fsyntax-only -std=c++2b "C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp"
C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:4:20: error: call to consteval function 'g' is not a
      constant expression
constexpr auto e = g();
                   ^
C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:4:20: note: pointer to a consteval declaration is not a
      constant expression
C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:1:15: note: declared here
consteval int f1() { return 0; }
              ^
C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:4:16: error: constexpr variable 'e' must be initialized
      by a constant expression
constexpr auto e = g();
               ^   ~~~
C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:4:16: note: pointer to a consteval declaration is not a
      constant expression
C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:1:15: note: declared here
consteval int f1() { return 0; }
              ^
C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:5:21: error: cannot take address of consteval function
      'f1' outside of an immediate invocation
constexpr auto e1 = f1;
                    ^
C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:1:15: note: declared here
consteval int f1() { return 0; }
              ^
C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:5:16: error: constexpr variable 'e1' must be initialized
      by a constant expression
constexpr auto e1 = f1;
               ^    ~~
C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:5:16: note: pointer to a consteval declaration is not a
      constant expression
C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:1:15: note: declared here
consteval int f1() { return 0; }
              ^
4 errors generated.

These look like valid errors to me, so am I misunderstanding something? Is the concern that we're emitting error: constexpr variable '<whatever>' must be initialized by a constant expression after we already issued a diagnostic on the same line?

In D74130#3073271, @aaron.ballman wrote:

@Tyker -- are you planning to pick this review back up again sometime in the near future? If not, do you care if the review gets commandeered?

here is an update that i think is close to committable. if it is not i am fine if it gets commandeered.

LGTM

seems good to me. but maybe other still have comments.

In D98228#2624345, @aqjune wrote:

In D98228#2613304, @Tyker wrote:

I think the process of checking is a value is noundef needs to be more general than just calls. maybe addKnowledge would be a better place.
because nonnull and align information can also be inferred from loads/stores.

Hi, sorry for my delay in response.
Could you elaborate a bit with an example, please?
Does it mean that the updated analysis may return suboptimal results?

In D90529#2619492, @aqjune wrote:

In D90529#2613303, @Tyker wrote:

In D90529#2612794, @aqjune wrote:

A fix in AssumeBundleBuilder to make it comply LangRef: D98228

BTW, I found that "align" can take two operands: "align"(i8* ptr, i64 a, i64 b) What is the meaning of the second index (b)?

the meaning of the second index is the offset of the alignment. so with "align"(i8* ptr, i64 16, i64 12), ptr is aligned on 4 but ptr + 4 is aligned on 16.
I introduced this to support __builtin_assume_aligned which has similar semantics.

Hi, thanks for the info.

But I think an example in Transforms/AlignmentFromAssumptions/simple.ll conflicts with your definition. It has:

28 define i32 @foo2a(i32* nocapture %a) nounwind uwtable readonly {
29 entry:
30   tail call void @llvm.assume(i1 true) ["align"(i32* %a, i32 32, i32 28)]
31   %arrayidx = getelementptr inbounds i32, i32* %a, i64 -1
32   %0 = load i32, i32* %arrayidx, align 4
33   ret i32 %0
34 
35 ; CHECK-LABEL: @foo2a
36 ; CHECK: load i32, i32* {{[^,]+}}, align 32
37 ; CHECK: ret i32
38 }

"align"(i32* %a, i32 32, i32 28) means %a + 4 is 32-bytes aligned, IIUC. Then, %a - 4 cannot be 32-bytes aligned, is it?

Yes, this looks like a bug.

I think the process of checking is a value is noundef needs to be more general than just calls. maybe addKnowledge would be a better place.
because nonnull and align information can also be inferred from loads/stores.

In D90529#2612794, @aqjune wrote:

A fix in AssumeBundleBuilder to make it comply LangRef: D98228

BTW, I found that "align" can take two operands: "align"(i8* ptr, i64 a, i64 b) What is the meaning of the second index (b)?

In D96646#2594843, @andrew.w.kaylor wrote:

I'm not trying to be difficult, but I genuinely still don't understand the additional arguments pointer. Is it intended to allow proprietary extensions? Is there an example somewhere?

If these intrinsics are meant as a general mechanism to enable arbitrary communication between custom front ends and custom optimization passes, that's fine. I'd just like to see something explicitly explaining that.

added more details about the storage of additional arguments.

fixed the sentence.

addressed comment

This seems good to me but i am not familiar the the auto-upgrader.

I added the changes to the langref in https://reviews.llvm.org/D96646

rebased, addressed comments. fixed issues.

In D82703#2489196, @jdoerfert wrote:

@Tyker are you going to upstream this? If not, can someone take over?

In rGd093401a2617d3c46aaed9eeaecf877e3ae1a9f1#958951, @aaron.ballman wrote:

Just an FYI, but this probably should have gotten a review as it's not NFC (it changes what information gets dumped in text and JSON form, impacts AST matching behavior, and other minor effects). That said, overall the direction seems fine to me, though I did question one of the changes.

I recently made it much easier to create AnnotationAttr in this context with https://reviews.llvm.org/rGd093401a2617d3c46aaed9eeaecf877e3ae1a9f1.

In D88645#2353152, @thakis wrote:

Looks like this broke tests: http://45.33.8.238/linux/31159/step_12.txt

Please take a look, and revert for now if it takes a while to fix.

this is fixed by 4afa077899b