Skip to content

Make clause payloads separator-aware and keep clause occurrences#13

Merged
ouankou merged 6 commits into
mainfrom
issue-12-structured-payloads
Nov 24, 2025
Merged

Make clause payloads separator-aware and keep clause occurrences#13
ouankou merged 6 commits into
mainfrom
issue-12-structured-payloads

Conversation

@ouankou
Copy link
Copy Markdown
Owner

@ouankou ouankou commented Nov 24, 2025

Summary

  • Replace stringly clause payloads with structured types: add OpenACCExpressionItem/OpenACCIdentifier, make async/default_async IDs, wait devnum/queues IDs, gang args, device/num_gangs/num_workers/device_num/tile/vector/vector_length/worker lengths, if/self conditions, routine/bind/cache vars, and data/reduction var lists store text+separator (no post-unparse hacks).
  • Update parser actions and merge helpers to build/compare structured payloads directly; keep clause merging off by default for round-trip, but enable merging in built-in test harness to validate normalized output without spacing hacks.
  • Ensure merged var lists/devices/use_device deduplicate entries when merging is on; add use_device exit hook and dedup logic for generic expression lists under merging.
  • Restore built-in reference outputs from main (normalized, merged form) and keep VV round-trip outputs unchanged; add clause-merging-aware dedup to preserve normalized behavior.
  • CI: add concurrency guard to accparser workflow to prevent overlapping runs on the same ref.

Testing

  • cmake --build build
  • ctest --output-on-failure}

@ouankou
Copy link
Copy Markdown
Owner Author

ouankou commented Nov 24, 2025

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Hooray!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@ouankou
Copy link
Copy Markdown
Owner Author

ouankou commented Nov 24, 2025

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🎉

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@ouankou ouankou force-pushed the issue-12-structured-payloads branch from 5845f06 to 1e482bb Compare November 24, 2025 05:11
@ouankou
Copy link
Copy Markdown
Owner Author

ouankou commented Nov 24, 2025

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@ouankou
Copy link
Copy Markdown
Owner Author

ouankou commented Nov 24, 2025

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/OpenACCIR.h
Comment on lines +118 to +121
void addVar(const std::string &expr,
OpenACCClauseSeparator sep = ACCC_CLAUSE_SEP_comma) {
vars.push_back(OpenACCExpressionItem{expr, sep});
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Deduplicate var-list entries when merging clauses

When clause merging is enabled, var-list clauses are still fed through the addVar(const std::string&, …) overload, which at lines 118‑121 just appends each item and never checks for duplicates. In merging mode the parser reuses the first clause instance (e.g., two copy(a) clauses end up calling this overload twice on the same object), so the normalized output now becomes copy(a, a) instead of the previously deduped copy(a). This regresses the merge/normalization behavior even when OpenACCDirective::setClauseMerging(true) is set.

Useful? React with 👍 / 👎.

@ouankou
Copy link
Copy Markdown
Owner Author

ouankou commented Nov 24, 2025

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Chef's kiss.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@ouankou ouankou merged commit 31efa63 into main Nov 24, 2025
11 checks passed
@ouankou ouankou deleted the issue-12-structured-payloads branch November 24, 2025 06:21
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.

1 participant