Skip to content
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.

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

[SYCL] Updating program compile, link and build options per spec #71

Closed
wants to merge 4 commits into from

Conversation

schittir
Copy link
Contributor

clarifications

Signed-off-by: Sindhu Chittireddy [email protected]

Copy link
Contributor

@vladimirlaz vladimirlaz left a comment

Choose a reason for hiding this comment

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

LGTM

@bader bader closed this Apr 16, 2019
@bader
Copy link
Contributor

bader commented Apr 16, 2019

This PR looks empty. Not sure why. I guess it's because of "force-pushes".
@schittir, suggest using unique branch for each pull request.

iclsrc pushed a commit that referenced this pull request Nov 2, 2023
…vg (#70914)

This removes explicit invalidation of vg and svg that was done in
`GDBRemoteRegisterContext::AArch64Reconfigure`. This was in fact
covering up a bug elsehwere.

Register information says that a write to vg also invalidates svg (it
does not unless you are in streaming mode, but we decided to keep it
simple and say it always does).

This invalidation was not being applied until *after* AArch64Reconfigure
was called. This meant that without those manual invalidates this
happened:
* vg is written
* svg is not invalidated
* Reconfigure uses the written vg value
* Reconfigure uses the *old* svg value

I have moved the AArch64Reconfigure call to after we've processed the
invalidations caused by the register write, so we no longer need the
manual invalidates in AArch64Reconfigure.

In addition I have changed the order in which expedited registers as
parsed. These registers come with a stop notification and include,
amongst others, vg and svg.

So now we:
* Parse them and update register values (including vg and svg)
* AArch64Reconfigure, which uses those values, and invalidates every
register, because offsets may have changed.
* Parse the expedited registers again, knowing that none of the values
will have changed due to the scaling.

This means we use the expedited registers during the reconfigure, but
the invalidate does not mean we throw all of them away.

The cost is we parse them twice client side, but this is cheap compared
to a network packet, and is limited to AArch64 targets only.

On a system with SVE and SME, these are the packets sent for a step:
```
(lldb) b-remote.async>  < 803> read packet:
$T05thread:p1f80.1f80;name:main.o;threads:1f80;thread-pcs:000000000040056c<...>a1:0800000000000000;d9:0400000000000000;reason:trace;#fc
intern-state     <  21> send packet: $xfffffffff200,200#5e
intern-state     < 516> read packet:
$e4f2ffffffff000000<...>#71
intern-state     <  15> send packet: $Z0,400568,4#4d
intern-state     <   6> read packet: $OK#9a
dbg.evt-handler  <  16> send packet: $jThreadsInfo#c1
dbg.evt-handler  < 224> read packet:
$[{"name":"main.o","reason":"trace","registers":{"161":"0800000000000000",<...>}],"signal":5,"tid":8064}]]#73
```

You can see there are no extra register reads which means we're using
the expedited registers.

For a write to vg:
```
(lldb) register write vg 4
lldb             <  37> send packet:
$Pa1=0400000000000000;thread:1f80;#4a
lldb             <   6> read packet: $OK#9a
lldb             <  20> send packet: $pa1;thread:1f80;#29
lldb             <  20> read packet: $0400000000000000#04
lldb             <  20> send packet: $pd9;thread:1f80;#34
lldb             <  20> read packet: $0400000000000000#04
```

There is the initial P write, and lldb correctly assumes that SVG is
invalidated by this also so we read back the new vg and svg values
afterwards.
iclsrc pushed a commit that referenced this pull request Dec 21, 2023
… (#76167)

…396)"

This reverts commit 8773c9b.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants