Skip to content

Conversation

@zahiraam
Copy link
Contributor

@zahiraam zahiraam commented Aug 14, 2024

These two options were implemented to solve two issues (missing checksum for the main file on Windows and host compilation DICompileUnit pointing to a temporary file instead of the original source file). The introduction of the -include-footer solves these issues. These options can now be removed.

@zahiraam zahiraam marked this pull request as ready for review August 15, 2024 13:40
@zahiraam zahiraam requested review from a team as code owners August 15, 2024 13:40
@zahiraam zahiraam changed the title Remove -full-main-file-name and -fsycl-use-main-file-name options. [SYCL] Remove -full-main-file-name and -fsycl-use-main-file-name options. Aug 15, 2024
@zahiraam zahiraam requested a review from stevemerr August 15, 2024 13:43
@zahiraam
Copy link
Contributor Author

@stevemerr I have done some local test including the one I mentioned to you offline and it's passing. I think it would be nice to have you test full DBG testing to see if this change works for you. Thanks.

@stevemerr stevemerr requested a review from bwyma August 15, 2024 14:15
Copy link
Contributor

@mdtoguchi mdtoguchi left a comment

Choose a reason for hiding this comment

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

Cleanup OK for driver

Copy link
Contributor

@elizabethandrews elizabethandrews left a comment

Choose a reason for hiding this comment

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

FE cleanup looks ok to me. However, I am not very familiar with this functionality. Please wait for approval from @premanandrao and/or someone from the debug team

Copy link
Contributor

@bwyma bwyma left a comment

Choose a reason for hiding this comment

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

LGTM.

@zahiraam
Copy link
Contributor Author

zahiraam commented Sep 3, 2024

@premanandrao Can you please review this? Thanks.

Copy link
Contributor

@premanandrao premanandrao left a comment

Choose a reason for hiding this comment

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

FE changes LGTM

@zahiraam
Copy link
Contributor Author

zahiraam commented Sep 4, 2024

@intel/llvm-gatekeepers Can this be merged please? Thanks.

@martygrant martygrant merged commit 364d9ca into intel:sycl Sep 4, 2024
iclsrc pushed a commit that referenced this pull request Oct 14, 2025
Added Pattern for lowering `Math::ClampFOp` to `ROCDL::FMED3`.
Also added `chipset` option to `MathToRocdl` pass to check for arch
support ISA instructions

Solves [#15072](llvm/llvm-project#157052)

Reapplies llvm/llvm-project#160100

---------

Signed-off-by: Keshav Vinayak Jha <keshavvinayakjha@gmail.com>
iclsrc pushed a commit that referenced this pull request Oct 17, 2025
Added Pattern for lowering `Math::ClampFOp` to `ROCDL::FMED3`.
Also added `chipet` option to `MathToRocdl` pass to check for arch
support ISA instructions

Solves [#15072](llvm/llvm-project#157052)

Reapplies llvm/llvm-project#160100

Un-reverts the merged llvm/llvm-project#163259,
and fixes the error.

---------

Signed-off-by: Keshav Vinayak Jha <keshavvinayakjha@gmail.com>
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.

7 participants