Skip to content

Fix formatting in 2 files. Fixes #2 #3

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Fix formatting in 2 files. Fixes #2 #3

wants to merge 1 commit into from

Conversation

ssarangi
Copy link

@ssarangi ssarangi commented Feb 2, 2018

No description provided.

AnupamaChandrasekhar
AnupamaChandrasekhar reviewed Feb 6, 2018
View reviewed changes
Copy link
Contributor

@AnupamaChandrasekhar AnupamaChandrasekhar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi: I have reviewed your files and submitted it into our internal codebase for validation. It will be pushed to github after validation. Thanks for your contribution.

@AnupamaChandrasekhar
Copy link
Contributor

This change has been merged.

VPG-SWE-Github pushed a commit that referenced this pull request Nov 2, 2020
@igcbot @Dimus77
Revert of change: 242d131 Internal feature (#3)
a4d4b89 
Co-authored-by: Dmitry Kostin <[email protected]>
VPG-SWE-Github pushed a commit that referenced this pull request Nov 3, 2020
@igcbot @Dimus77
Revert of change: 242d131 Internal feature (#3)
6834cbe 
Co-authored-by: Dmitry Kostin <[email protected]>
VPG-SWE-Github pushed a commit that referenced this pull request Nov 3, 2020
@Dimus77 @jwarumze
Revert of change: 242d131 Internal feature (#3)
ae522a6
VPG-SWE-Github pushed a commit that referenced this pull request Nov 3, 2020
@Dimus77 @igcbot
Revert of change: 242d131 Internal feature (#3)
7adbe2f
VPG-SWE-Github pushed a commit that referenced this pull request Nov 3, 2020
@Dimus77 @igcbot
Revert of change: 242d131 Internal feature (#3)
12fcf2e
VPG-SWE-Github pushed a commit that referenced this pull request Nov 5, 2020
@Dimus77 @igcbot
Revert of change: 242d131 Internal feature (#3)
2195ed3
VPG-SWE-Github pushed a commit that referenced this pull request May 5, 2021
@dlei6g @igcbot
Bug fixes to enable function pointers directly passed by FE (Commit a…
d102c7f 
…ttempt #3)
VPG-SWE-Github pushed a commit that referenced this pull request Sep 13, 2021
@aparshin-intel @igcbot
fix generation of DWARF locations for indirect values
75f1453 
The problem comes from the inconsistency between semantics of <llvm.dbg.value>
and location descriptors defined by DWARF. <llvm.dbg.value> describes the
actual *value*, of a source variable - on the other hand, DWARF location
descriptors focus primarily (with few excepions) on describing the *location*
from which the value can be fetched (the location can be an address
in register file or memory).

Now, DWARF allows for the following location desciptors for the pieces of
variables:
    1. empty location
    2. register location
    3. memory location
    4. implicit location

The situation that was not processed correctly looked like this:

```
call void @llvm.dbg.value(metadata [4096 x i32] addrspace(1)* %0,
                          metadata !1299,
                          metadata !DIExpression(DW_OP_deref))
```

This means that the actual *value* of the source varible !1299 is located
at the address defined by %0. Let's assume %0 is held in a register - then
a proper DWARF location descripor should be of "memory location" type which is
evaluated to the value held in %0. In general case, the respected location
descripor should look like this:

```
    0: DW_OP_reg<N + 16>
    1: DW_OP_const1u <offset in GRF N>
    2: DW_OP_const1u <size in bits of %0>
    3: DW_OP_INTEL_push_bit_piece_stack
```

NOTE_1: Item #3, transforms the original "register location" defined by Item #0
to a "memory location". Such transformations are not possible in the vanilla
DWARF spec, thus we had to opt for intel extensions.

NOTE_2: It seems that we have some level of ambiguity. For example,
I think it is perfectly legal to have this kind of expressions:

```
call void @llvm.dbg.value(metadata i32 %0,
                          metadata !1299,
                          metadata !DIExpression(DW_OP_deref), DW_OP_plus_uconst, 3)
```

Such an expression defines a concrete value. But our current implementaion won't
be able to handle this. From dwarf point of view the respected expression
should be an implicit value. While in reality our backend will emit a
<memory location expression>.
I've added some asserts and diagnostics to detect such cases.
VPG-SWE-Github pushed a commit that referenced this pull request Sep 28, 2021
@fangliu2020 @igcbot
[vISA]: Fix the WAW/WAR issue of ACC caused by mul/mulh/madw in loca…
3caf520 
…l scheduling. #3 try.

on some platforms, mul/mulh/madw will be expanded into mul+mach/macl after local scheduling.
Since ACC is used as dst of expanded mul, it is a must to set implicit ACC as dst
of mul/mulh/madw. Otherwise, there will be WAW/WAR issue of ACC in local scheduling.
VPG-SWE-Github pushed a commit that referenced this pull request Oct 19, 2021
@fangliu2020 @igcbot
Enable madw use in i64 mul emulator. #3 try.
ad8248c 
Enable madw use in i64 mul emulator. #3 try.
VPG-SWE-Github pushed a commit that referenced this pull request Oct 21, 2021
@aparshin-intel @igcbot
fixed loading of constant structs by ConstantLoader
12bcdd3 
constant returns are not allowed, and PostLegalization is expected to
insert a proper constant loading sequence. However, if we return a
constant structure, before this patch the generated sequence looked like
this:

```
ret { <4 x i32> } { <4 x i32> <i32 1, i32 1, i32 1, i32 1> })
```
->
```
%const_val = call { <4 x i32> } @llvm.genx.constanti.s1v4i32(
            { <4 x i32> } { <4 x i32> <i32 1, i32 1, i32 1, i32 1> })
ret %const_val
```

Issue: *genx.constanti* is designed to handle proper vector types, not
aggregates!

This commit introduces the following changes:
1. All constant structures are treated as non-simple constants and
always loaded by loadNonSimpleConstants
2. Undef values are left as is
3. ConstantLoader class must not be used with aggregate types (including
structs).

NOTE_1: In theory, restriction #3 can be relaxed by allowing
ConstantLoader to operate on such types. 2 minor issues are prevent us
from  doing that:
I. I haven't observed such cases in practice :).
II. This would require us to handle *undef* values. The resulting code
looks extremely quirky as the constant loader must produce instruction,
not a value.

NOTE_2: This change may not solve all the remaining issues with constant
structures. For example - loadPhiConstants could potentially encounter a
constant struct. If this happens in practice - then we must
relax our restrictions on ConsantLoader (see NOTE_1).
VPG-SWE-Github pushed a commit that referenced this pull request Oct 22, 2021
@igorban-intel @igcbot
Fix generation of variable location descriptor for llvm.dbg.declare
1ffd2e9 
For support llvm.declare - it is necessary to make the correct
sequence to access the memory location. In base implementation
for llvm.declare generates sequence
```
	0: DW_OP_breg[x]
	1: DW_OP_const1u
	2: DW_OP_const1u
	3: DW_OP_INTEL_push_bit_piece_stack
```
DW_OP_breg[x] - was generated by method addRegisterLoc. But in
intel DWARF extensions - Item #3, transforms the original
"register location" defined by Item #0 to a "memory location".
And this sequence becomes incorrect, because DW_OP_breg is
addressing operation, which pushed on stack  contents of a
register(but here expecting register location).

Here operations DW_OP_breg[x] is replaced for DW_OP_reg[x],
that returns register location for reg.
Sequence for llvm.declare after current change will be:
```
        0: DW_OP_reg[x]
        1: DW_OP_const1u
        2: DW_OP_const1u
        3: DW_OP_INTEL_push_bit_piece_stack
```
@haonanya haonanya mentioned this pull request Jan 25, 2022
Closed
VPG-SWE-Github pushed a commit that referenced this pull request Apr 25, 2023
@dlei6g @igcbot
Disable ForceInlineStackCallWithImplArg by default (Try #3)
1c33b7c 
Disable ForceInlineStackCallWithImplArg since Implicit Arg buffer for stackcalls (i.e. EnableGlobalStateBuffer) has been implemented and enabled.
VPG-SWE-Github pushed a commit that referenced this pull request May 18, 2023
@dlei6g @igcbot
Re-enable FunctionCloningThreshold for ZeBin (Try #3)
533fe1b 
FunctionCloningThreshold was disabled due to regressions on ZeBin enabling.
Re-enable again after fixes for:
* Implicit Arg bug for LocalID
* Incorrect vISA scratch space calculation for indirect calls
VPG-SWE-Github pushed a commit that referenced this pull request May 23, 2023
@dlei6g @igcbot
Only enable CallWA for SIMD32 when nested stackcalls or indirect cal…
3a4cff2 
…ls are present (Try #3)

Only enable CallWA for SIMD32 when nested stackcalls or indirect calls are present.
Also added FunctionGroupAnalysis Function Group attributes: hasSubroutine() and isIndirectCallGroup(), plus general refactor for FGA and CShader.
VPG-SWE-Github pushed a commit that referenced this pull request May 16, 2025
@bokrzesi @igcbot
Fixing ArgSize calculation for metadata type function argument in Op…
7a45c84 
…enCLPasses -> PrivateMemoryResolution and fix of invalid "sret" LegalizeFunctionSignatures void to struct transformation

In such case

```
...
call void @llvm.experimental.noalias.scope.decl(metadata !805)
...

; Function Attrs: nounwind
declare spir_func %structtype @__devicelib_catanf(%structtype sret(%structtype) align 4 %0) #2

; Function Attrs: nocallback nofree nosync nounwind willreturn memory(inaccessiblemem: readwrite)
declare void @llvm.experimental.noalias.scope.decl(metadata %0) #3

!805 = !{!806}
!806 = distinct !{!806, !807, !"catanf: argument 0"}
!807 = distinct !{!807, !"catanf"}
```

When PrivateMemoryResolution was "recursively calculating the max private mem usage of all callees"
Then during processing function argument that is MetadataTy it was failing due to

```
Assertion failed: Ty->isSized() && "Cannot getTypeInfo() on a type that is unsized!",
file ...\dump64\llvm-deps-16.0.6\src\llvm\lib\IR\DataLayout.cpp, line 751
````

So I added check for MetadataTy which skips it.

Additionally `LegalizeFunctionSignatures` was transforming the return type of void function that used sret() as its argument.
The LLVM IR verifier didnt like that since the docs state that:
`A function that accepts an sret argument must return void. A return value may not be sret.`

I've decided to perform skip on sret cases in that pass.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AnupamaChandrasekhar AnupamaChandrasekhar AnupamaChandrasekhar left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ssarangi @AnupamaChandrasekhar